[PATCH v3 0/3] Add SD/MMC support for Renesas RZ/V2H(P) SoC

[PATCH v3 0/3] Add SD/MMC support for Renesas RZ/V2H(P) SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi All,

This patch series aims to add SD/MMC support for Renesas RZ/V2H(P) SoC.

Below is the sample usage of using internal regulator:

SoC DTSI node:
sdhi1: mmc@15c10000 {
        compatible = "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 167>,
                        <&cpg CPG_MOD 169>,
                        <&cpg CPG_MOD 168>,
                        <&cpg CPG_MOD 170>;
        clock-names = "core", "clkh", "cd", "aclk";
        resets = <&cpg 168>;
        power-domains = <&cpg>;
        status = "disabled";

        vqmmc_sdhi1: vqmmc-regulator {
                regulator-compatible = "vqmmc-r9a09g057-regulator";
                regulator-name = "sdhi1-vqmmc-regulator";
                regulator-min-microvolt = <1800000>;
                regulator-max-microvolt = <3300000>;
                status = "disabled";
        };
};

* Example of SDHI1 while using internal regulator:
** Board DTS:
&sdhi1 {
        pinctrl-0 = <&sdhi1_pins>;
        pinctrl-1 = <&sdhi1_pins>;
        pinctrl-names = "default", "state_uhs";
        renesas,sdhi-use-internal-regulator;
        vmmc-supply = <&reg_3p3v>;
        vqmmc-supply = <&vqmmc_sdhi1>;
        bus-width = <4>;
        sd-uhs-sdr50;
        sd-uhs-sdr104;
        status = "okay";
};

&vqmmc_sdhi1 {
	status = "okay";
};


* Example of SDHI1 while using GPIO regulator while internal regulator is present:
** Board DTS:
vccq_sdhi1: regulator-vccq-sdhi1 {
        compatible = "regulator-gpio";
        regulator-name = "SDHI1 VccQ";
        regulator-min-microvolt = <1800000>;
        regulator-max-microvolt = <3300000>;
        gpios = <&pinctrl RZG2L_GPIO(10, 2) GPIO_ACTIVE_HIGH>;
        gpios-states = <0>;
        states = <3300000 0>, <1800000 1>;
};

&sdhi1 {
        pinctrl-0 = <&sdhi1_pins>;
        pinctrl-1 = <&sdhi1_pins>;
        pinctrl-names = "default", "state_uhs";
        vmmc-supply = <&reg_3p3v>;
        vqmmc-supply = <&vccq_sdhi1>;
        bus-width = <4>;
        sd-uhs-sdr50;
        sd-uhs-sdr104;
        status = "okay";
};

v2->v3
- Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
- Added regulator-compatible property for vqmmc-regulator
- Added 'renesas,sdhi-use-internal-regulator' DT property
- Included RB tags for patch 2/3
- Moved regulator info to renesas_sdhi_of_data instead of quirks
- Added support to configure the init state of regulator
- Added function pointers to configure regulator
- Added REGULATOR_CHANGE_VOLTAGE mask

v1->v2
- Dropped regulator core API changes
- Updated DT binding
- Now controlling PWEN bit via regultor api

v1: https://patchwork.kernel.org/project/linux-renesas-soc/cover/20240605074936.578687-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Cheers,
Prabhakar

Lad Prabhakar (3):
  dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support
  mmc: tmio: Use MMC core APIs to control the vqmmc regulator
  mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

 .../devicetree/bindings/mmc/renesas,sdhi.yaml |  30 +++-
 drivers/mmc/host/renesas_sdhi.h               |  13 ++
 drivers/mmc/host/renesas_sdhi_core.c          |  93 +++++++++++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 150 ++++++++++++++++++
 drivers/mmc/host/tmio_mmc.h                   |   5 +
 drivers/mmc/host/tmio_mmc_core.c              |   7 +-
 6 files changed, 293 insertions(+), 5 deletions(-)

-- 
2.34.1

[PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that
of the R-Car Gen3, but it has some differences:
- HS400 is not supported.
- It supports the SD_IOVS bit to control the IO voltage level.
- It supports fixed address mode.

To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
compatible string is added.

A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN)
and voltage level switching for the SD/MMC.

Additionally, the 'renesas,sdhi-use-internal-regulator' flag is introduced
to indicate that an internal regulator is used instead of a
GPIO-controlled regulator. This flag will help configure the internal
regulator and avoid special handling when GPIO is used for voltage
regulation instead of the SD_(IOVS/PWEN) pins.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2->v3
- Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
- Added regulator-compatible property for vqmmc-regulator
- Added 'renesas,sdhi-use-internal-regulator' property

v1->v2
- Moved vqmmc object in the if block
- Updated commit message
---
 .../devicetree/bindings/mmc/renesas,sdhi.yaml | 30 ++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
index 3d0e61e59856..20769434a422 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
+++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
@@ -18,6 +18,7 @@ properties:
           - renesas,sdhi-r7s9210 # SH-Mobile AG5
           - renesas,sdhi-r8a73a4 # R-Mobile APE6
           - renesas,sdhi-r8a7740 # R-Mobile A1
+          - renesas,sdhi-r9a09g057 # RZ/V2H(P)
           - renesas,sdhi-sh73a0  # R-Mobile APE6
       - items:
           - enum:
@@ -118,7 +119,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: renesas,rzg2l-sdhi
+            enum:
+              - renesas,sdhi-r9a09g057
+              - renesas,rzg2l-sdhi
     then:
       properties:
         clocks:
@@ -204,6 +207,31 @@ 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-r9a09g057
+    then:
+      properties:
+        renesas,sdhi-use-internal-regulator:
+          $ref: /schemas/types.yaml#/definitions/flag
+          description:
+            Flag to indicate internal regulator is being used instead of GPIO regulator.
+
+        vqmmc-regulator:
+          type: object
+          description: VQMMC SD regulator
+          $ref: /schemas/regulator/regulator.yaml#
+          unevaluatedProperties: false
+
+          properties:
+            regulator-compatible:
+              pattern: "^vqmmc-r9a09g057-regulator"
+
+      required:
+        - vqmmc-regulator
+
 required:
   - compatible
   - reg
-- 
2.34.1

Re: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Conor Dooley]

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

On Mon, Jun 24, 2024 at 04:32:27PM +0100, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that
> of the R-Car Gen3, but it has some differences:
> - HS400 is not supported.
> - It supports the SD_IOVS bit to control the IO voltage level.
> - It supports fixed address mode.
> 
> To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> compatible string is added.
> 
> A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN)
> and voltage level switching for the SD/MMC.
> 
> Additionally, the 'renesas,sdhi-use-internal-regulator' flag is introduced
> to indicate that an internal regulator is used instead of a
> GPIO-controlled regulator. This flag will help configure the internal
> regulator and avoid special handling when GPIO is used for voltage
> regulation instead of the SD_(IOVS/PWEN) pins.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v2->v3
> - Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
> - Added regulator-compatible property for vqmmc-regulator
> - Added 'renesas,sdhi-use-internal-regulator' property
> 
> v1->v2
> - Moved vqmmc object in the if block
> - Updated commit message
> ---
>  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 30 ++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> index 3d0e61e59856..20769434a422 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> @@ -18,6 +18,7 @@ properties:
>            - renesas,sdhi-r7s9210 # SH-Mobile AG5
>            - renesas,sdhi-r8a73a4 # R-Mobile APE6
>            - renesas,sdhi-r8a7740 # R-Mobile A1
> +          - renesas,sdhi-r9a09g057 # RZ/V2H(P)
>            - renesas,sdhi-sh73a0  # R-Mobile APE6
>        - items:
>            - enum:
> @@ -118,7 +119,9 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: renesas,rzg2l-sdhi
> +            enum:
> +              - renesas,sdhi-r9a09g057
> +              - renesas,rzg2l-sdhi
>      then:
>        properties:
>          clocks:
> @@ -204,6 +207,31 @@ 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-r9a09g057
> +    then:
> +      properties:

Please define properties at the top level and constrain then per
compatible.

Thanks,
Conor.

> +        renesas,sdhi-use-internal-regulator:
> +          $ref: /schemas/types.yaml#/definitions/flag
> +          description:
> +            Flag to indicate internal regulator is being used instead of GPIO regulator.
> +
> +        vqmmc-regulator:
> +          type: object
> +          description: VQMMC SD regulator
> +          $ref: /schemas/regulator/regulator.yaml#
> +          unevaluatedProperties: false
> +
> +          properties:
> +            regulator-compatible:
> +              pattern: "^vqmmc-r9a09g057-regulator"
> +
> +      required:
> +        - vqmmc-regulator
> +
>  required:
>    - compatible
>    - reg
> -- 
> 2.34.1
> 

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

Re: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Lad, Prabhakar]

Hi Conor,

Thank you for the review.

On Mon, Jun 24, 2024 at 5:39 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Mon, Jun 24, 2024 at 04:32:27PM +0100, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that
> > of the R-Car Gen3, but it has some differences:
> > - HS400 is not supported.
> > - It supports the SD_IOVS bit to control the IO voltage level.
> > - It supports fixed address mode.
> >
> > To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> > compatible string is added.
> >
> > A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN)
> > and voltage level switching for the SD/MMC.
> >
> > Additionally, the 'renesas,sdhi-use-internal-regulator' flag is introduced
> > to indicate that an internal regulator is used instead of a
> > GPIO-controlled regulator. This flag will help configure the internal
> > regulator and avoid special handling when GPIO is used for voltage
> > regulation instead of the SD_(IOVS/PWEN) pins.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v2->v3
> > - Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
> > - Added regulator-compatible property for vqmmc-regulator
> > - Added 'renesas,sdhi-use-internal-regulator' property
> >
> > v1->v2
> > - Moved vqmmc object in the if block
> > - Updated commit message
> > ---
> >  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 30 ++++++++++++++++++-
> >  1 file changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > index 3d0e61e59856..20769434a422 100644
> > --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > @@ -18,6 +18,7 @@ properties:
> >            - renesas,sdhi-r7s9210 # SH-Mobile AG5
> >            - renesas,sdhi-r8a73a4 # R-Mobile APE6
> >            - renesas,sdhi-r8a7740 # R-Mobile A1
> > +          - renesas,sdhi-r9a09g057 # RZ/V2H(P)
> >            - renesas,sdhi-sh73a0  # R-Mobile APE6
> >        - items:
> >            - enum:
> > @@ -118,7 +119,9 @@ allOf:
> >        properties:
> >          compatible:
> >            contains:
> > -            const: renesas,rzg2l-sdhi
> > +            enum:
> > +              - renesas,sdhi-r9a09g057
> > +              - renesas,rzg2l-sdhi
> >      then:
> >        properties:
> >          clocks:
> > @@ -204,6 +207,31 @@ 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-r9a09g057
> > +    then:
> > +      properties:
>
> Please define properties at the top level and constrain then per
> compatible.
>
Ok, I'll move them to the top level.

Cheers,
Prabhakar

Re: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Geert Uytterhoeven]

Hi Prabhakar,

On Mon, Jun 24, 2024 at 5:33 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that
> of the R-Car Gen3, but it has some differences:
> - HS400 is not supported.
> - It supports the SD_IOVS bit to control the IO voltage level.
> - It supports fixed address mode.
>
> To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> compatible string is added.
>
> A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN)
> and voltage level switching for the SD/MMC.
>
> Additionally, the 'renesas,sdhi-use-internal-regulator' flag is introduced
> to indicate that an internal regulator is used instead of a
> GPIO-controlled regulator. This flag will help configure the internal
> regulator and avoid special handling when GPIO is used for voltage
> regulation instead of the SD_(IOVS/PWEN) pins.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v2->v3
> - Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
> - Added regulator-compatible property for vqmmc-regulator
> - Added 'renesas,sdhi-use-internal-regulator' property

Thanks for the update!

> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> @@ -204,6 +207,31 @@ 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-r9a09g057
> +    then:
> +      properties:
> +        renesas,sdhi-use-internal-regulator:
> +          $ref: /schemas/types.yaml#/definitions/flag
> +          description:
> +            Flag to indicate internal regulator is being used instead of GPIO regulator.

Do you really need this?
The status of the regulator subnode already indicates this.

> +
> +        vqmmc-regulator:
> +          type: object
> +          description: VQMMC SD regulator
> +          $ref: /schemas/regulator/regulator.yaml#
> +          unevaluatedProperties: false
> +
> +          properties:
> +            regulator-compatible:
> +              pattern: "^vqmmc-r9a09g057-regulator"
> +
> +      required:
> +        - vqmmc-regulator

I'm not 100% sure this works correctly: does the checker complain if
a required subnode is disabled? Note that I haven't checked that.

> +
>  required:
>    - compatible
>    - reg

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 v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Lad, Prabhakar]

Hi Geert,

Thank you for the review.

On Tue, Jun 25, 2024 at 7:57 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, Jun 24, 2024 at 5:33 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that
> > of the R-Car Gen3, but it has some differences:
> > - HS400 is not supported.
> > - It supports the SD_IOVS bit to control the IO voltage level.
> > - It supports fixed address mode.
> >
> > To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> > compatible string is added.
> >
> > A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN)
> > and voltage level switching for the SD/MMC.
> >
> > Additionally, the 'renesas,sdhi-use-internal-regulator' flag is introduced
> > to indicate that an internal regulator is used instead of a
> > GPIO-controlled regulator. This flag will help configure the internal
> > regulator and avoid special handling when GPIO is used for voltage
> > regulation instead of the SD_(IOVS/PWEN) pins.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v2->v3
> > - Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
> > - Added regulator-compatible property for vqmmc-regulator
> > - Added 'renesas,sdhi-use-internal-regulator' property
>
> Thanks for the update!
>
> > --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > @@ -204,6 +207,31 @@ 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-r9a09g057
> > +    then:
> > +      properties:
> > +        renesas,sdhi-use-internal-regulator:
> > +          $ref: /schemas/types.yaml#/definitions/flag
> > +          description:
> > +            Flag to indicate internal regulator is being used instead of GPIO regulator.
>
> Do you really need this?
For cases where the status is okay for the regulator but still the
user has phandle for the GPIO regulator or shall I drop this case?

> The status of the regulator subnode already indicates this.
You mean to use of_device_is_available() ?

>
> > +
> > +        vqmmc-regulator:
> > +          type: object
> > +          description: VQMMC SD regulator
> > +          $ref: /schemas/regulator/regulator.yaml#
> > +          unevaluatedProperties: false
> > +
> > +          properties:
> > +            regulator-compatible:
> > +              pattern: "^vqmmc-r9a09g057-regulator"
> > +
> > +      required:
> > +        - vqmmc-regulator
>
> I'm not 100% sure this works correctly: does the checker complain if
> a required subnode is disabled? Note that I haven't checked that.
>
Here is the experiment which I tried and the checker didnt complain,

&sdhi1 {
    status = "okay";
};

&vqmmc_sdhi1 {
    status = "disabled";
};

But the above is still a valid case where the user wants to use a GPIO
regulator?

Cheers,
Prabhakar

Re: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Geert Uytterhoeven]

Hi Prabhakar,

On Tue, Jun 25, 2024 at 10:47 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Tue, Jun 25, 2024 at 7:57 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > On Mon, Jun 24, 2024 at 5:33 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that
> > > of the R-Car Gen3, but it has some differences:
> > > - HS400 is not supported.
> > > - It supports the SD_IOVS bit to control the IO voltage level.
> > > - It supports fixed address mode.
> > >
> > > To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> > > compatible string is added.
> > >
> > > A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN)
> > > and voltage level switching for the SD/MMC.
> > >
> > > Additionally, the 'renesas,sdhi-use-internal-regulator' flag is introduced
> > > to indicate that an internal regulator is used instead of a
> > > GPIO-controlled regulator. This flag will help configure the internal
> > > regulator and avoid special handling when GPIO is used for voltage
> > > regulation instead of the SD_(IOVS/PWEN) pins.
> > >
> > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > > v2->v3
> > > - Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
> > > - Added regulator-compatible property for vqmmc-regulator
> > > - Added 'renesas,sdhi-use-internal-regulator' property
> >
> > Thanks for the update!
> >
> > > --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > > +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > > @@ -204,6 +207,31 @@ 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-r9a09g057
> > > +    then:
> > > +      properties:
> > > +        renesas,sdhi-use-internal-regulator:
> > > +          $ref: /schemas/types.yaml#/definitions/flag
> > > +          description:
> > > +            Flag to indicate internal regulator is being used instead of GPIO regulator.
> >
> > Do you really need this?
> For cases where the status is okay for the regulator but still the
> user has phandle for the GPIO regulator or shall I drop this case?

I think that case can be ignored.
The regulator subnode would be disabled by default in the .dtsi, right?

> > The status of the regulator subnode already indicates this.
> You mean to use of_device_is_available() ?

Exactly. I.e. only register the regulator when it is enabled.

> > > +
> > > +        vqmmc-regulator:
> > > +          type: object
> > > +          description: VQMMC SD regulator
> > > +          $ref: /schemas/regulator/regulator.yaml#
> > > +          unevaluatedProperties: false
> > > +
> > > +          properties:
> > > +            regulator-compatible:
> > > +              pattern: "^vqmmc-r9a09g057-regulator"
> > > +
> > > +      required:
> > > +        - vqmmc-regulator
> >
> > I'm not 100% sure this works correctly: does the checker complain if
> > a required subnode is disabled? Note that I haven't checked that.
> >
> Here is the experiment which I tried and the checker didnt complain,
>
> &sdhi1 {
>     status = "okay";
> };
>
> &vqmmc_sdhi1 {
>     status = "disabled";
> };

OK, thanks for checking!

> But the above is still a valid case where the user wants to use a GPIO
> regulator?

Yes it is.


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 v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Lad, Prabhakar]

Hi Geert,

On Tue, Jun 25, 2024 at 10:02 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Tue, Jun 25, 2024 at 10:47 AM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> > On Tue, Jun 25, 2024 at 7:57 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > On Mon, Jun 24, 2024 at 5:33 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > >
> > > > The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that
> > > > of the R-Car Gen3, but it has some differences:
> > > > - HS400 is not supported.
> > > > - It supports the SD_IOVS bit to control the IO voltage level.
> > > > - It supports fixed address mode.
> > > >
> > > > To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> > > > compatible string is added.
> > > >
> > > > A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN)
> > > > and voltage level switching for the SD/MMC.
> > > >
> > > > Additionally, the 'renesas,sdhi-use-internal-regulator' flag is introduced
> > > > to indicate that an internal regulator is used instead of a
> > > > GPIO-controlled regulator. This flag will help configure the internal
> > > > regulator and avoid special handling when GPIO is used for voltage
> > > > regulation instead of the SD_(IOVS/PWEN) pins.
> > > >
> > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > ---
> > > > v2->v3
> > > > - Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
> > > > - Added regulator-compatible property for vqmmc-regulator
> > > > - Added 'renesas,sdhi-use-internal-regulator' property
> > >
> > > Thanks for the update!
> > >
> > > > --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > > > +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > > > @@ -204,6 +207,31 @@ 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-r9a09g057
> > > > +    then:
> > > > +      properties:
> > > > +        renesas,sdhi-use-internal-regulator:
> > > > +          $ref: /schemas/types.yaml#/definitions/flag
> > > > +          description:
> > > > +            Flag to indicate internal regulator is being used instead of GPIO regulator.
> > >
> > > Do you really need this?
> > For cases where the status is okay for the regulator but still the
> > user has phandle for the GPIO regulator or shall I drop this case?
>
> I think that case can be ignored.
> The regulator subnode would be disabled by default in the .dtsi, right?
>
Yes, agreed.

> > > The status of the regulator subnode already indicates this.
> > You mean to use of_device_is_available() ?
>
> Exactly. I.e. only register the regulator when it is enabled.
>
Okay I'll use of_device_is_available() and drop the
'renesas,sdhi-use-internal-regulator' property.

Cheers,
Prabhakar

RE: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Biju Das]

Hi Prabhakar,

> -----Original Message-----
> From: Prabhakar <prabhakar.csengg@gmail.com>
> Sent: Monday, June 24, 2024 4:32 PM
> Subject: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support
> 
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that of the R-Car Gen3, but it
> has some differences:
> - HS400 is not supported.
> - It supports the SD_IOVS bit to control the IO voltage level.
> - It supports fixed address mode.
> 
> To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> compatible string is added.
> 
> A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN) and voltage level
> switching for the SD/MMC.
> 
> Additionally, the 'renesas,sdhi-use-internal-regulator' flag is introduced to indicate that an
> internal regulator is used instead of a GPIO-controlled regulator. This flag will help configure
> the internal regulator and avoid special handling when GPIO is used for voltage regulation instead
> of the SD_(IOVS/PWEN) pins.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v2->v3
> - Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
> - Added regulator-compatible property for vqmmc-regulator
> - Added 'renesas,sdhi-use-internal-regulator' property
> 
> v1->v2
> - Moved vqmmc object in the if block
> - Updated commit message
> ---
>  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 30 ++++++++++++++++++-
>  1 file changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> index 3d0e61e59856..20769434a422 100644
> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> @@ -18,6 +18,7 @@ properties:
>            - renesas,sdhi-r7s9210 # SH-Mobile AG5
>            - renesas,sdhi-r8a73a4 # R-Mobile APE6
>            - renesas,sdhi-r8a7740 # R-Mobile A1
> +          - renesas,sdhi-r9a09g057 # RZ/V2H(P)
>            - renesas,sdhi-sh73a0  # R-Mobile APE6
>        - items:
>            - enum:
> @@ -118,7 +119,9 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: renesas,rzg2l-sdhi
> +            enum:
> +              - renesas,sdhi-r9a09g057
> +              - renesas,rzg2l-sdhi
>      then:
>        properties:
>          clocks:
> @@ -204,6 +207,31 @@ 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-r9a09g057
> +    then:
> +      properties:
> +        renesas,sdhi-use-internal-regulator:
> +          $ref: /schemas/types.yaml#/definitions/flag
> +          description:
> +            Flag to indicate internal regulator is being used instead of GPIO regulator.
> +
> +        vqmmc-regulator:
> +          type: object
> +          description: VQMMC SD regulator
> +          $ref: /schemas/regulator/regulator.yaml#
> +          unevaluatedProperties: false
> +
> +          properties:
> +            regulator-compatible:
> +              pattern: "^vqmmc-r9a09g057-regulator"
> +
> +      required:
> +        - vqmmc-regulator

Maybe we can drop required to make it optional, so that one has the
option to select between { vqmmc-regulator, gpio regulator}??

Cheers,
Biju

Re: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Lad, Prabhakar]

Hi Biju,

On Tue, Jun 25, 2024 at 10:12 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
> Hi Prabhakar,
>
> > -----Original Message-----
> > From: Prabhakar <prabhakar.csengg@gmail.com>
> > Sent: Monday, June 24, 2024 4:32 PM
> > Subject: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support
> >
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to that of the R-Car Gen3, but it
> > has some differences:
> > - HS400 is not supported.
> > - It supports the SD_IOVS bit to control the IO voltage level.
> > - It supports fixed address mode.
> >
> > To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> > compatible string is added.
> >
> > A 'vqmmc-regulator' object is introduced to handle the power enable (PWEN) and voltage level
> > switching for the SD/MMC.
> >
> > Additionally, the 'renesas,sdhi-use-internal-regulator' flag is introduced to indicate that an
> > internal regulator is used instead of a GPIO-controlled regulator. This flag will help configure
> > the internal regulator and avoid special handling when GPIO is used for voltage regulation instead
> > of the SD_(IOVS/PWEN) pins.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v2->v3
> > - Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
> > - Added regulator-compatible property for vqmmc-regulator
> > - Added 'renesas,sdhi-use-internal-regulator' property
> >
> > v1->v2
> > - Moved vqmmc object in the if block
> > - Updated commit message
> > ---
> >  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 30 ++++++++++++++++++-
> >  1 file changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > index 3d0e61e59856..20769434a422 100644
> > --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > @@ -18,6 +18,7 @@ properties:
> >            - renesas,sdhi-r7s9210 # SH-Mobile AG5
> >            - renesas,sdhi-r8a73a4 # R-Mobile APE6
> >            - renesas,sdhi-r8a7740 # R-Mobile A1
> > +          - renesas,sdhi-r9a09g057 # RZ/V2H(P)
> >            - renesas,sdhi-sh73a0  # R-Mobile APE6
> >        - items:
> >            - enum:
> > @@ -118,7 +119,9 @@ allOf:
> >        properties:
> >          compatible:
> >            contains:
> > -            const: renesas,rzg2l-sdhi
> > +            enum:
> > +              - renesas,sdhi-r9a09g057
> > +              - renesas,rzg2l-sdhi
> >      then:
> >        properties:
> >          clocks:
> > @@ -204,6 +207,31 @@ 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-r9a09g057
> > +    then:
> > +      properties:
> > +        renesas,sdhi-use-internal-regulator:
> > +          $ref: /schemas/types.yaml#/definitions/flag
> > +          description:
> > +            Flag to indicate internal regulator is being used instead of GPIO regulator.
> > +
> > +        vqmmc-regulator:
> > +          type: object
> > +          description: VQMMC SD regulator
> > +          $ref: /schemas/regulator/regulator.yaml#
> > +          unevaluatedProperties: false
> > +
> > +          properties:
> > +            regulator-compatible:
> > +              pattern: "^vqmmc-r9a09g057-regulator"
> > +
> > +      required:
> > +        - vqmmc-regulator
>
> Maybe we can drop required to make it optional, so that one has the
> option to select between { vqmmc-regulator, gpio regulator}??
>
I think making the regulator node optional isn't correct here as this
internal regulator is always present in the SoC and this has to be
described in the DT no matter if it's being used or not.

Users can always switch between internal regulator and GPIO regulator.
I have provided an example here [0] for both the cases.

[0] https://patchwork.kernel.org/project/linux-renesas-soc/cover/20240624153229.68882-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Cheers,
Prabhakar

RE: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Biju Das]

> -----Original Message-----
> From: Lad, Prabhakar <prabhakar.csengg@gmail.com>
> Sent: Tuesday, June 25, 2024 10:19 AM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>; Wolfram Sang <wsa+renesas@sang-engineering.com>; Geert
> Uytterhoeven <geert+renesas@glider.be>; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Magnus Damm <magnus.damm@gmail.com>;
> linux-mmc@vger.kernel.org; devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-renesas-
> soc@vger.kernel.org; Fabrizio Castro <fabrizio.castro.jz@renesas.com>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Subject: Re: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support
> 
> Hi Biju,
> 
> On Tue, Jun 25, 2024 at 10:12 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > Hi Prabhakar,
> >
> > > -----Original Message-----
> > > From: Prabhakar <prabhakar.csengg@gmail.com>
> > > Sent: Monday, June 24, 2024 4:32 PM
> > > Subject: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document
> > > RZ/V2H(P) support
> > >
> > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > >
> > > The SD/MMC block on the RZ/V2H(P) ("R9A09G057") SoC is similar to
> > > that of the R-Car Gen3, but it has some differences:
> > > - HS400 is not supported.
> > > - It supports the SD_IOVS bit to control the IO voltage level.
> > > - It supports fixed address mode.
> > >
> > > To accommodate these differences, a SoC-specific 'renesas,sdhi-r9a09g057'
> > > compatible string is added.
> > >
> > > A 'vqmmc-regulator' object is introduced to handle the power enable
> > > (PWEN) and voltage level switching for the SD/MMC.
> > >
> > > Additionally, the 'renesas,sdhi-use-internal-regulator' flag is
> > > introduced to indicate that an internal regulator is used instead of
> > > a GPIO-controlled regulator. This flag will help configure the
> > > internal regulator and avoid special handling when GPIO is used for voltage regulation instead
> of the SD_(IOVS/PWEN) pins.
> > >
> > > Signed-off-by: Lad Prabhakar
> > > <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > ---
> > > v2->v3
> > > - Renamed vqmmc-r9a09g057-regulator object to vqmmc-regulator
> > > - Added regulator-compatible property for vqmmc-regulator
> > > - Added 'renesas,sdhi-use-internal-regulator' property
> > >
> > > v1->v2
> > > - Moved vqmmc object in the if block
> > > - Updated commit message
> > > ---
> > >  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 30
> > > ++++++++++++++++++-
> > >  1 file changed, 29 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > > b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > > index 3d0e61e59856..20769434a422 100644
> > > --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > > +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > > @@ -18,6 +18,7 @@ properties:
> > >            - renesas,sdhi-r7s9210 # SH-Mobile AG5
> > >            - renesas,sdhi-r8a73a4 # R-Mobile APE6
> > >            - renesas,sdhi-r8a7740 # R-Mobile A1
> > > +          - renesas,sdhi-r9a09g057 # RZ/V2H(P)
> > >            - renesas,sdhi-sh73a0  # R-Mobile APE6
> > >        - items:
> > >            - enum:
> > > @@ -118,7 +119,9 @@ allOf:
> > >        properties:
> > >          compatible:
> > >            contains:
> > > -            const: renesas,rzg2l-sdhi
> > > +            enum:
> > > +              - renesas,sdhi-r9a09g057
> > > +              - renesas,rzg2l-sdhi
> > >      then:
> > >        properties:
> > >          clocks:
> > > @@ -204,6 +207,31 @@ 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-r9a09g057
> > > +    then:
> > > +      properties:
> > > +        renesas,sdhi-use-internal-regulator:
> > > +          $ref: /schemas/types.yaml#/definitions/flag
> > > +          description:
> > > +            Flag to indicate internal regulator is being used instead of GPIO regulator.
> > > +
> > > +        vqmmc-regulator:
> > > +          type: object
> > > +          description: VQMMC SD regulator
> > > +          $ref: /schemas/regulator/regulator.yaml#
> > > +          unevaluatedProperties: false
> > > +
> > > +          properties:
> > > +            regulator-compatible:
> > > +              pattern: "^vqmmc-r9a09g057-regulator"
> > > +
> > > +      required:
> > > +        - vqmmc-regulator
> >
> > Maybe we can drop required to make it optional, so that one has the
> > option to select between { vqmmc-regulator, gpio regulator}??
> >
> I think making the regulator node optional isn't correct here as this internal regulator is always
> present in the SoC and this has to be described in the DT no matter if it's being used or not.

Agreed, but user can make it optional by setting pinmux as gpio and 
the internal regulator is valid only if we make it as a function.

From, SoC point vqmmc-regulator is always available. So, it needs to be described
as above. But required property and disabled in the node somewhat confusing??

Cheers,
Biju

Re: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P) support

[Lad, Prabhakar]

On Tue, Jun 25, 2024 at 10:47 AM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
<snip>
> > > > v1->v2
> > > > - Moved vqmmc object in the if block
> > > > - Updated commit message
> > > > ---
> > > >  .../devicetree/bindings/mmc/renesas,sdhi.yaml | 30
> > > > ++++++++++++++++++-
> > > >  1 file changed, 29 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > > > b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > > > index 3d0e61e59856..20769434a422 100644
> > > > --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > > > +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> > > > @@ -18,6 +18,7 @@ properties:
> > > >            - renesas,sdhi-r7s9210 # SH-Mobile AG5
> > > >            - renesas,sdhi-r8a73a4 # R-Mobile APE6
> > > >            - renesas,sdhi-r8a7740 # R-Mobile A1
> > > > +          - renesas,sdhi-r9a09g057 # RZ/V2H(P)
> > > >            - renesas,sdhi-sh73a0  # R-Mobile APE6
> > > >        - items:
> > > >            - enum:
> > > > @@ -118,7 +119,9 @@ allOf:
> > > >        properties:
> > > >          compatible:
> > > >            contains:
> > > > -            const: renesas,rzg2l-sdhi
> > > > +            enum:
> > > > +              - renesas,sdhi-r9a09g057
> > > > +              - renesas,rzg2l-sdhi
> > > >      then:
> > > >        properties:
> > > >          clocks:
> > > > @@ -204,6 +207,31 @@ 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-r9a09g057
> > > > +    then:
> > > > +      properties:
> > > > +        renesas,sdhi-use-internal-regulator:
> > > > +          $ref: /schemas/types.yaml#/definitions/flag
> > > > +          description:
> > > > +            Flag to indicate internal regulator is being used instead of GPIO regulator.
> > > > +
> > > > +        vqmmc-regulator:
> > > > +          type: object
> > > > +          description: VQMMC SD regulator
> > > > +          $ref: /schemas/regulator/regulator.yaml#
> > > > +          unevaluatedProperties: false
> > > > +
> > > > +          properties:
> > > > +            regulator-compatible:
> > > > +              pattern: "^vqmmc-r9a09g057-regulator"
> > > > +
> > > > +      required:
> > > > +        - vqmmc-regulator
> > >
> > > Maybe we can drop required to make it optional, so that one has the
> > > option to select between { vqmmc-regulator, gpio regulator}??
> > >
> > I think making the regulator node optional isn't correct here as this internal regulator is always
> > present in the SoC and this has to be described in the DT no matter if it's being used or not.
>
> Agreed, but user can make it optional by setting pinmux as gpio and
> the internal regulator is valid only if we make it as a function.
>
> From, SoC point vqmmc-regulator is always available. So, it needs to be described
> as above. But required property and disabled in the node somewhat confusing??
>
'required' here is mainly to enforce validation for the checkers.
Maybe the DT maintainers can better explain here...

Cheers,
Prabhakar

[PATCH v3 2/3] mmc: tmio: Use MMC core APIs to control the vqmmc regulator

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Use the mmc_regulator_enable_vqmmc() and mmc_regulator_disable_vqmmc() APIs
to enable/disable the vqmmc regulator.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2->v3
- Included RB tags

v1->v2
- New patch
---
 drivers/mmc/host/tmio_mmc_core.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 93e912afd3ae..2ec1a74c85bc 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -897,8 +897,8 @@ static void tmio_mmc_power_on(struct tmio_mmc_host *host, unsigned short vdd)
 	 * It seems, VccQ should be switched on after Vcc, this is also what the
 	 * omap_hsmmc.c driver does.
 	 */
-	if (!IS_ERR(mmc->supply.vqmmc) && !ret) {
-		ret = regulator_enable(mmc->supply.vqmmc);
+	if (!ret) {
+		ret = mmc_regulator_enable_vqmmc(mmc);
 		usleep_range(200, 300);
 	}
 
@@ -911,8 +911,7 @@ static void tmio_mmc_power_off(struct tmio_mmc_host *host)
 {
 	struct mmc_host *mmc = host->mmc;
 
-	if (!IS_ERR(mmc->supply.vqmmc))
-		regulator_disable(mmc->supply.vqmmc);
+	mmc_regulator_disable_vqmmc(mmc);
 
 	if (!IS_ERR(mmc->supply.vmmc))
 		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);
-- 
2.34.1

Re: [PATCH v3 2/3] mmc: tmio: Use MMC core APIs to control the vqmmc regulator

[claudiu beznea]

On 24.06.2024 18:32, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Use the mmc_regulator_enable_vqmmc() and mmc_regulator_disable_vqmmc() APIs
> to enable/disable the vqmmc regulator.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>\

Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> # on RZ/G3S

> ---
> v2->v3
> - Included RB tags
> 
> v1->v2
> - New patch
> ---
>  drivers/mmc/host/tmio_mmc_core.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index 93e912afd3ae..2ec1a74c85bc 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -897,8 +897,8 @@ static void tmio_mmc_power_on(struct tmio_mmc_host *host, unsigned short vdd)
>  	 * It seems, VccQ should be switched on after Vcc, this is also what the
>  	 * omap_hsmmc.c driver does.
>  	 */
> -	if (!IS_ERR(mmc->supply.vqmmc) && !ret) {
> -		ret = regulator_enable(mmc->supply.vqmmc);
> +	if (!ret) {
> +		ret = mmc_regulator_enable_vqmmc(mmc);
>  		usleep_range(200, 300);
>  	}
>  
> @@ -911,8 +911,7 @@ static void tmio_mmc_power_off(struct tmio_mmc_host *host)
>  {
>  	struct mmc_host *mmc = host->mmc;
>  
> -	if (!IS_ERR(mmc->supply.vqmmc))
> -		regulator_disable(mmc->supply.vqmmc);
> +	mmc_regulator_disable_vqmmc(mmc);
>  
>  	if (!IS_ERR(mmc->supply.vmmc))
>  		mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);

[PATCH v3 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

[Prabhakar]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

The SDHI/eMMC IPs found in the RZ/V2H(P) (a.k.a. r9a09g057) are very
similar to those found in R-Car Gen3. However, they are not identical,
necessitating an SoC-specific compatible string for fine-tuning driver
support.

Key features of the RZ/V2H(P) SDHI/eMMC IPs include:
- Voltage level control via the IOVS bit.
- PWEN pin support via SD_STATUS register.
- Lack of HS400 support.
- Fixed address mode operation.

internal regulator support is added to control the voltage levels of SD
pins via sd_iovs/sd_pwen bits in SD_STATUS register.

As the users can use GPIO regulator instead of SD_IOVS/PWEN pins
'renesas,sdhi-use-internal-regulator' DT property is used for
special handling of internal regulator.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v2->v3
- Moved regulator info to renesas_sdhi_of_data instead of quirks
- Added support to configure the init state of regulator
- Added function pointers to configure regulator
- Added REGULATOR_CHANGE_VOLTAGE mask

v1->v2
- Now controlling PWEN bit get/set_voltage
---
 drivers/mmc/host/renesas_sdhi.h               |  13 ++
 drivers/mmc/host/renesas_sdhi_core.c          |  93 +++++++++++
 drivers/mmc/host/renesas_sdhi_internal_dmac.c | 150 ++++++++++++++++++
 drivers/mmc/host/tmio_mmc.h                   |   5 +
 4 files changed, 261 insertions(+)

diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
index 586f94d4dbfd..377804bbc37e 100644
--- a/drivers/mmc/host/renesas_sdhi.h
+++ b/drivers/mmc/host/renesas_sdhi.h
@@ -11,6 +11,8 @@
 
 #include <linux/dmaengine.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
 #include "tmio_mmc.h"
 
 struct renesas_sdhi_scc {
@@ -35,6 +37,12 @@ struct renesas_sdhi_of_data {
 	unsigned int max_blk_count;
 	unsigned short max_segs;
 	unsigned long sdhi_flags;
+	struct regulator_desc *rdesc;
+	struct regulator_init_data *reg_init_data;
+	bool regulator_init_state;
+	unsigned int regulator_init_voltage;
+	int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
+	int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned int voltage);
 };
 
 #define SDHI_CALIB_TABLE_MAX 32
@@ -93,6 +101,11 @@ struct renesas_sdhi {
 	unsigned int tap_set;
 
 	struct reset_control *rstc;
+
+	bool use_internal_regulator;
+	struct regulator_dev *internal_regulator;
+	int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
+	int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned int voltage);
 };
 
 #define host_to_priv(host) \
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
index 12f4faaaf4ee..8cf912512f07 100644
--- a/drivers/mmc/host/renesas_sdhi_core.c
+++ b/drivers/mmc/host/renesas_sdhi_core.c
@@ -581,12 +581,50 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host, bool preserve)
 
 	if (!preserve) {
 		if (priv->rstc) {
+			bool regulator_enabled;
+
+			/* to restore back the internal regulator after reset make a copy of it */
+			if (priv->use_internal_regulator)
+				regulator_enabled = regulator_is_enabled(host->mmc->supply.vqmmc);
+
 			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->use_internal_regulator) {
+				int voltage;
+
+				/*
+				 * HW reset might have toggled the regulator state in HW
+				 * which regulator core might be unaware of so restore
+				 * back the regulator state in HW bypassing the regulator
+				 * core.
+				 */
+				if (regulator_enabled !=
+				    regulator_is_enabled(host->mmc->supply.vqmmc)) {
+					ret = priv->regulator_force_endis(priv->internal_regulator,
+									  regulator_enabled);
+					if (ret)
+						dev_err(&host->pdev->dev,
+							"Failed to %s internal regulator\n",
+							regulator_enabled ? "enable" : "disable");
+				}
+
+				/* restore back voltage on vqmmc supply */
+				voltage = regulator_get_voltage(host->mmc->supply.vqmmc);
+				if (voltage != host->mmc->ios.signal_voltage) {
+					voltage = host->mmc->ios.signal_voltage ==
+						  MMC_SIGNAL_VOLTAGE_330 ? 3300000 : 1800000;
+					ret = regulator_set_voltage(host->mmc->supply.vqmmc,
+								    voltage, voltage);
+					if (ret)
+						dev_err(&host->pdev->dev,
+							"Failed to set voltage on internal regulator\n");
+				}
+			}
+
 			priv->needs_adjust_hs400 = false;
 			renesas_sdhi_set_clock(host, host->clk_cache);
 
@@ -904,6 +942,29 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
 	renesas_sdhi_sdbuf_width(host, enable ? width : 16);
 }
 
+static int renesas_sdhi_internal_dmac_register_regulator(struct platform_device *pdev,
+							 const struct renesas_sdhi_of_data *of_data)
+{
+	struct tmio_mmc_host *host = platform_get_drvdata(pdev);
+	struct renesas_sdhi *priv = host_to_priv(host);
+	struct regulator_config rcfg = {
+		.dev = &pdev->dev,
+		.driver_data = host,
+		.init_data = of_data->reg_init_data,
+	};
+	const char *devname;
+
+	devname = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-vqmmc-regulator",
+				 dev_name(&pdev->dev));
+	if (!devname)
+		return -ENOMEM;
+
+	of_data->rdesc->name = devname;
+	priv->internal_regulator = devm_regulator_register(&pdev->dev, of_data->rdesc, &rcfg);
+
+	return PTR_ERR_OR_ZERO(priv->internal_regulator);
+}
+
 int renesas_sdhi_probe(struct platform_device *pdev,
 		       const struct tmio_mmc_dma_ops *dma_ops,
 		       const struct renesas_sdhi_of_data *of_data,
@@ -970,6 +1031,10 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	if (IS_ERR(host))
 		return PTR_ERR(host);
 
+	if (pdev->dev.of_node)
+		priv->use_internal_regulator = of_property_read_bool(pdev->dev.of_node,
+								     "renesas,sdhi-use-internal-regulator");
+
 	if (of_data) {
 		mmc_data->flags |= of_data->tmio_flags;
 		mmc_data->ocr_mask = of_data->tmio_ocr_mask;
@@ -980,6 +1045,18 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 		mmc_data->max_segs = of_data->max_segs;
 		dma_priv->dma_buswidth = of_data->dma_buswidth;
 		host->bus_shift = of_data->bus_shift;
+		if (priv->use_internal_regulator) {
+			if (!of_data->regulator_force_endis)
+				return dev_err_probe(&pdev->dev, -EINVAL,
+					"missing function pointer to force regulator enable/disable");
+			priv->regulator_force_endis =
+				of_data->regulator_force_endis;
+			if (!of_data->regulator_force_voltage)
+				return dev_err_probe(&pdev->dev, -EINVAL,
+					"missing function pointer to force regulator voltage");
+			priv->regulator_force_voltage =
+				of_data->regulator_force_voltage;
+		}
 		/* Fallback for old DTs */
 		if (!priv->clkh && of_data->sdhi_flags & SDHI_FLAG_NEED_CLKH_FALLBACK)
 			priv->clkh = clk_get_parent(clk_get_parent(priv->clk));
@@ -1051,6 +1128,22 @@ int renesas_sdhi_probe(struct platform_device *pdev,
 	if (ret)
 		goto efree;
 
+	if (priv->use_internal_regulator && of_data) {
+		ret = renesas_sdhi_internal_dmac_register_regulator(pdev, of_data);
+		if (ret)
+			goto efree;
+
+		/* Set the default init state for regulator in the HW */
+		ret = priv->regulator_force_endis(priv->internal_regulator,
+						  of_data->regulator_init_state);
+		if (ret)
+			goto efree;
+		ret = priv->regulator_force_voltage(priv->internal_regulator,
+						    of_data->regulator_init_voltage);
+		if (ret)
+			goto efree;
+	}
+
 	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/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
index 422fa63a2e99..a70a1ae6919c 100644
--- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
+++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
@@ -89,6 +89,150 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
 	},
 };
 
+static const unsigned int r9a09g057_vqmmc_voltages[] = {
+	1800000, 3300000,
+};
+
+static int r9a09g057_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 r9a09g057_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 r9a09g057_regulator_force_endis(struct regulator_dev *rdev, bool enable)
+{
+	if (enable)
+		return r9a09g057_regulator_enable(rdev);
+
+	return r9a09g057_regulator_disable(rdev);
+}
+
+static int r9a09g057_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);
+	if (sd_status & SD_STATUS_PWEN)
+		return 1;
+
+	return 0;
+}
+
+static int r9a09g057_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);
+	if (sd_status & SD_STATUS_IOVS)
+		return 1800000;
+
+	return 3300000;
+}
+
+static int r9a09g057_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 = 0;
+	} else {
+		sd_status &=  ~SD_STATUS_IOVS;
+		*selector = 1;
+	}
+	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
+
+	return 0;
+}
+
+static int r9a09g057_regulator_force_voltage(struct regulator_dev *rdev,
+					     unsigned int voltage)
+{
+	unsigned int selector = 0;
+
+	return r9a09g057_regulator_set_voltage(rdev, voltage, voltage, &selector);
+}
+
+static int r9a09g057_regulator_list_voltage(struct regulator_dev *rdev,
+					    unsigned int selector)
+{
+	if (selector >= ARRAY_SIZE(r9a09g057_vqmmc_voltages))
+		return -EINVAL;
+
+	return r9a09g057_vqmmc_voltages[selector];
+}
+
+static struct regulator_init_data r9a09g057_regulator_init_data = {
+	.constraints = {
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS | REGULATOR_CHANGE_VOLTAGE,
+	},
+};
+
+static const struct regulator_ops r9a09g057_regulator_voltage_ops = {
+	.enable = r9a09g057_regulator_enable,
+	.disable = r9a09g057_regulator_disable,
+	.is_enabled = r9a09g057_regulator_is_enabled,
+	.list_voltage = r9a09g057_regulator_list_voltage,
+	.get_voltage = r9a09g057_regulator_get_voltage,
+	.set_voltage = r9a09g057_regulator_set_voltage,
+	.map_voltage = regulator_map_voltage_ascend,
+};
+
+static struct regulator_desc r9a09g057_vqmmc_regulator = {
+	.of_match	= of_match_ptr("vqmmc-r9a09g057-regulator"),
+	.owner		= THIS_MODULE,
+	.type		= REGULATOR_VOLTAGE,
+	.ops		= &r9a09g057_regulator_voltage_ops,
+	.volt_table	= r9a09g057_vqmmc_voltages,
+	.n_voltages	= ARRAY_SIZE(r9a09g057_vqmmc_voltages),
+};
+
+static const struct renesas_sdhi_of_data of_data_r9a09g057 = {
+	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
+			  TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
+	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
+			  MMC_CAP_CMD23 | MMC_CAP_WAIT_WHILE_BUSY,
+	.capabilities2	= MMC_CAP2_NO_WRITE_PROTECT | MMC_CAP2_MERGE_CAPABLE,
+	.bus_shift	= 2,
+	.scc_offset	= 0x1000,
+	.taps		= rcar_gen3_scc_taps,
+	.taps_num	= ARRAY_SIZE(rcar_gen3_scc_taps),
+	/* DMAC can handle 32bit blk count but only 1 segment */
+	.max_blk_count	= UINT_MAX / TMIO_MAX_BLK_SIZE,
+	.max_segs	= 1,
+	.sdhi_flags	= SDHI_FLAG_NEED_CLKH_FALLBACK,
+	.rdesc = &r9a09g057_vqmmc_regulator,
+	.reg_init_data = &r9a09g057_regulator_init_data,
+	.regulator_init_state = false,
+	.regulator_init_voltage = 3300000,
+	.regulator_force_endis = r9a09g057_regulator_force_endis,
+	.regulator_force_voltage = r9a09g057_regulator_force_voltage,
+};
+
 static const struct renesas_sdhi_of_data of_data_rza2 = {
 	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
 			  TMIO_MMC_HAVE_CBSY,
@@ -260,6 +404,11 @@ static const struct renesas_sdhi_of_data_with_quirks of_rzg2l_compatible = {
 	.quirks = &sdhi_quirks_rzg2l,
 };
 
+static const struct renesas_sdhi_of_data_with_quirks of_r9a09g057_compatible = {
+	.of_data = &of_data_r9a09g057,
+	.quirks = &sdhi_quirks_rzg2l,
+};
+
 static const struct renesas_sdhi_of_data_with_quirks of_rcar_gen3_compatible = {
 	.of_data = &of_data_rcar_gen3,
 };
@@ -284,6 +433,7 @@ static const struct of_device_id renesas_sdhi_internal_dmac_of_match[] = {
 	{ .compatible = "renesas,sdhi-r8a77990", .data = &of_r8a77990_compatible, },
 	{ .compatible = "renesas,sdhi-r8a77995", .data = &of_rcar_gen3_nohs400_compatible, },
 	{ .compatible = "renesas,sdhi-r9a09g011", .data = &of_rzg2l_compatible, },
+	{ .compatible = "renesas,sdhi-r9a09g057", .data = &of_r9a09g057_compatible, },
 	{ .compatible = "renesas,rzg2l-sdhi", .data = &of_rzg2l_compatible, },
 	{ .compatible = "renesas,rcar-gen3-sdhi", .data = &of_rcar_gen3_compatible, },
 	{ .compatible = "renesas,rcar-gen4-sdhi", .data = &of_rcar_gen3_compatible, },
diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
index de56e6534aea..1a3172786662 100644
--- a/drivers/mmc/host/tmio_mmc.h
+++ b/drivers/mmc/host/tmio_mmc.h
@@ -43,6 +43,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 and RZ/V2H(P) */
 
 /* Definitions for values the CTL_STOP_INTERNAL_ACTION register can take */
 #define TMIO_STOP_STP		BIT(0)
@@ -102,6 +103,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/V2H(P) */
+#define SD_STATUS_IOVS		BIT(16) /* only known on RZ/V2H(P) */
+
 /* Define some IRQ masks */
 /* This is the mask used at reset by the chip */
 #define TMIO_MASK_ALL           0x837f031d
-- 
2.34.1

Re: [PATCH v3 3/3] mmc: renesas_sdhi: Add support for RZ/V2H(P) SoC

[claudiu beznea]

On 24.06.2024 18:32, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> The SDHI/eMMC IPs found in the RZ/V2H(P) (a.k.a. r9a09g057) are very
> similar to those found in R-Car Gen3. However, they are not identical,
> necessitating an SoC-specific compatible string for fine-tuning driver
> support.
> 
> Key features of the RZ/V2H(P) SDHI/eMMC IPs include:
> - Voltage level control via the IOVS bit.
> - PWEN pin support via SD_STATUS register.
> - Lack of HS400 support.
> - Fixed address mode operation.
> 
> internal regulator support is added to control the voltage levels of SD
> pins via sd_iovs/sd_pwen bits in SD_STATUS register.
> 
> As the users can use GPIO regulator instead of SD_IOVS/PWEN pins
> 'renesas,sdhi-use-internal-regulator' DT property is used for
> special handling of internal regulator.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> # on RZ/G3S

Some minor comments above. Feel free to address them or not.

Thank you,
Claudiu Beznea

> ---
> v2->v3
> - Moved regulator info to renesas_sdhi_of_data instead of quirks
> - Added support to configure the init state of regulator
> - Added function pointers to configure regulator
> - Added REGULATOR_CHANGE_VOLTAGE mask
> 
> v1->v2
> - Now controlling PWEN bit get/set_voltage
> ---
>  drivers/mmc/host/renesas_sdhi.h               |  13 ++
>  drivers/mmc/host/renesas_sdhi_core.c          |  93 +++++++++++
>  drivers/mmc/host/renesas_sdhi_internal_dmac.c | 150 ++++++++++++++++++
>  drivers/mmc/host/tmio_mmc.h                   |   5 +
>  4 files changed, 261 insertions(+)
> 
> diff --git a/drivers/mmc/host/renesas_sdhi.h b/drivers/mmc/host/renesas_sdhi.h
> index 586f94d4dbfd..377804bbc37e 100644
> --- a/drivers/mmc/host/renesas_sdhi.h
> +++ b/drivers/mmc/host/renesas_sdhi.h
> @@ -11,6 +11,8 @@
>  
>  #include <linux/dmaengine.h>
>  #include <linux/platform_device.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/regulator/machine.h>
>  #include "tmio_mmc.h"
>  
>  struct renesas_sdhi_scc {
> @@ -35,6 +37,12 @@ struct renesas_sdhi_of_data {
>  	unsigned int max_blk_count;
>  	unsigned short max_segs;
>  	unsigned long sdhi_flags;
> +	struct regulator_desc *rdesc;
> +	struct regulator_init_data *reg_init_data;
> +	bool regulator_init_state;
> +	unsigned int regulator_init_voltage;
> +	int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
> +	int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned int voltage);
>  };
>  
>  #define SDHI_CALIB_TABLE_MAX 32
> @@ -93,6 +101,11 @@ struct renesas_sdhi {
>  	unsigned int tap_set;
>  
>  	struct reset_control *rstc;
> +
> +	bool use_internal_regulator;
> +	struct regulator_dev *internal_regulator;
> +	int (*regulator_force_endis)(struct regulator_dev *rdev, bool enable);
> +	int (*regulator_force_voltage)(struct regulator_dev *rdev, unsigned int voltage);
>  };
>  
>  #define host_to_priv(host) \
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index 12f4faaaf4ee..8cf912512f07 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -581,12 +581,50 @@ static void renesas_sdhi_reset(struct tmio_mmc_host *host, bool preserve)
>  
>  	if (!preserve) {
>  		if (priv->rstc) {
> +			bool regulator_enabled;
> +
> +			/* to restore back the internal regulator after reset make a copy of it */
> +			if (priv->use_internal_regulator)
> +				regulator_enabled = regulator_is_enabled(host->mmc->supply.vqmmc);
> +
>  			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->use_internal_regulator) {
> +				int voltage;
> +
> +				/*
> +				 * HW reset might have toggled the regulator state in HW
> +				 * which regulator core might be unaware of so restore
> +				 * back the regulator state in HW bypassing the regulator
> +				 * core.
> +				 */
> +				if (regulator_enabled !=
> +				    regulator_is_enabled(host->mmc->supply.vqmmc)) {
> +					ret = priv->regulator_force_endis(priv->internal_regulator,
> +									  regulator_enabled);
> +					if (ret)
> +						dev_err(&host->pdev->dev,
> +							"Failed to %s internal regulator\n",
> +							regulator_enabled ? "enable" : "disable");
> +				}
> +
> +				/* restore back voltage on vqmmc supply */
> +				voltage = regulator_get_voltage(host->mmc->supply.vqmmc);
> +				if (voltage != host->mmc->ios.signal_voltage) {
> +					voltage = host->mmc->ios.signal_voltage ==
> +						  MMC_SIGNAL_VOLTAGE_330 ? 3300000 : 1800000;
> +					ret = regulator_set_voltage(host->mmc->supply.vqmmc,
> +								    voltage, voltage);
> +					if (ret)
> +						dev_err(&host->pdev->dev,
> +							"Failed to set voltage on internal regulator\n");
> +				}
> +			}
> +
>  			priv->needs_adjust_hs400 = false;
>  			renesas_sdhi_set_clock(host, host->clk_cache);
>  
> @@ -904,6 +942,29 @@ static void renesas_sdhi_enable_dma(struct tmio_mmc_host *host, bool enable)
>  	renesas_sdhi_sdbuf_width(host, enable ? width : 16);
>  }
>  
> +static int renesas_sdhi_internal_dmac_register_regulator(struct platform_device *pdev,
> +							 const struct renesas_sdhi_of_data *of_data)
> +{
> +	struct tmio_mmc_host *host = platform_get_drvdata(pdev);
> +	struct renesas_sdhi *priv = host_to_priv(host);
> +	struct regulator_config rcfg = {
> +		.dev = &pdev->dev,
> +		.driver_data = host,
> +		.init_data = of_data->reg_init_data,
> +	};
> +	const char *devname;
> +
> +	devname = devm_kasprintf(&pdev->dev, GFP_KERNEL, "%s-vqmmc-regulator",
> +				 dev_name(&pdev->dev));
> +	if (!devname)
> +		return -ENOMEM;
> +
> +	of_data->rdesc->name = devname;
> +	priv->internal_regulator = devm_regulator_register(&pdev->dev, of_data->rdesc, &rcfg);
> +
> +	return PTR_ERR_OR_ZERO(priv->internal_regulator);
> +}
> +
>  int renesas_sdhi_probe(struct platform_device *pdev,
>  		       const struct tmio_mmc_dma_ops *dma_ops,
>  		       const struct renesas_sdhi_of_data *of_data,
> @@ -970,6 +1031,10 @@ int renesas_sdhi_probe(struct platform_device *pdev,
>  	if (IS_ERR(host))
>  		return PTR_ERR(host);
>  
> +	if (pdev->dev.of_node)
> +		priv->use_internal_regulator = of_property_read_bool(pdev->dev.of_node,
> +								     "renesas,sdhi-use-internal-regulator");
> +
>  	if (of_data) {
>  		mmc_data->flags |= of_data->tmio_flags;
>  		mmc_data->ocr_mask = of_data->tmio_ocr_mask;
> @@ -980,6 +1045,18 @@ int renesas_sdhi_probe(struct platform_device *pdev,
>  		mmc_data->max_segs = of_data->max_segs;
>  		dma_priv->dma_buswidth = of_data->dma_buswidth;
>  		host->bus_shift = of_data->bus_shift;
> +		if (priv->use_internal_regulator) {
> +			if (!of_data->regulator_force_endis)
> +				return dev_err_probe(&pdev->dev, -EINVAL,
> +					"missing function pointer to force regulator enable/disable");
> +			priv->regulator_force_endis =
> +				of_data->regulator_force_endis;
> +			if (!of_data->regulator_force_voltage)
> +				return dev_err_probe(&pdev->dev, -EINVAL,
> +					"missing function pointer to force regulator voltage");
> +			priv->regulator_force_voltage =
> +				of_data->regulator_force_voltage;
> +		}
>  		/* Fallback for old DTs */
>  		if (!priv->clkh && of_data->sdhi_flags & SDHI_FLAG_NEED_CLKH_FALLBACK)
>  			priv->clkh = clk_get_parent(clk_get_parent(priv->clk));
> @@ -1051,6 +1128,22 @@ int renesas_sdhi_probe(struct platform_device *pdev,
>  	if (ret)
>  		goto efree;
>  
> +	if (priv->use_internal_regulator && of_data) {
> +		ret = renesas_sdhi_internal_dmac_register_regulator(pdev, of_data);
> +		if (ret)
> +			goto efree;
> +
> +		/* Set the default init state for regulator in the HW */
> +		ret = priv->regulator_force_endis(priv->internal_regulator,
> +						  of_data->regulator_init_state);
> +		if (ret)
> +			goto efree;
> +		ret = priv->regulator_force_voltage(priv->internal_regulator,
> +						    of_data->regulator_init_voltage);
> +		if (ret)
> +			goto efree;
> +	}
> +
>  	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/renesas_sdhi_internal_dmac.c b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> index 422fa63a2e99..a70a1ae6919c 100644
> --- a/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> +++ b/drivers/mmc/host/renesas_sdhi_internal_dmac.c
> @@ -89,6 +89,150 @@ static struct renesas_sdhi_scc rcar_gen3_scc_taps[] = {
>  	},
>  };
>  
> +static const unsigned int r9a09g057_vqmmc_voltages[] = {
> +	1800000, 3300000,
> +};
> +
> +static int r9a09g057_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;

There is an extra space after =

> +	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
> +
> +	return 0;
> +}
> +
> +static int r9a09g057_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;

Same here.

> +	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
> +
> +	return 0;
> +}
> +
> +static int r9a09g057_regulator_force_endis(struct regulator_dev *rdev, bool enable)
> +{
> +	if (enable)
> +		return r9a09g057_regulator_enable(rdev);
> +
> +	return r9a09g057_regulator_disable(rdev);
> +}
> +
> +static int r9a09g057_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);
> +	if (sd_status & SD_STATUS_PWEN)
> +		return 1;
> +
> +	return 0;

Could be re-written as:

	return !!(sd_status & SD_STATUS_PWEN)

> +}
> +
> +static int r9a09g057_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);
> +	if (sd_status & SD_STATUS_IOVS)
> +		return 1800000;
> +
> +	return 3300000;
> +}
> +
> +static int r9a09g057_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 = 0;
> +	} else {
> +		sd_status &=  ~SD_STATUS_IOVS;
> +		*selector = 1;
> +	}
> +	sd_ctrl_write32(host, CTL_SD_STATUS, sd_status);
> +
> +	return 0;
> +}
> +
> +static int r9a09g057_regulator_force_voltage(struct regulator_dev *rdev,
> +					     unsigned int voltage)
> +{
> +	unsigned int selector = 0;
> +
> +	return r9a09g057_regulator_set_voltage(rdev, voltage, voltage, &selector);
> +}
> +
> +static int r9a09g057_regulator_list_voltage(struct regulator_dev *rdev,
> +					    unsigned int selector)
> +{
> +	if (selector >= ARRAY_SIZE(r9a09g057_vqmmc_voltages))
> +		return -EINVAL;
> +
> +	return r9a09g057_vqmmc_voltages[selector];
> +}
> +
> +static struct regulator_init_data r9a09g057_regulator_init_data = {
> +	.constraints = {
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS | REGULATOR_CHANGE_VOLTAGE,
> +	},
> +};
> +
> +static const struct regulator_ops r9a09g057_regulator_voltage_ops = {
> +	.enable = r9a09g057_regulator_enable,
> +	.disable = r9a09g057_regulator_disable,
> +	.is_enabled = r9a09g057_regulator_is_enabled,
> +	.list_voltage = r9a09g057_regulator_list_voltage,
> +	.get_voltage = r9a09g057_regulator_get_voltage,
> +	.set_voltage = r9a09g057_regulator_set_voltage,
> +	.map_voltage = regulator_map_voltage_ascend,
> +};
> +
> +static struct regulator_desc r9a09g057_vqmmc_regulator = {
> +	.of_match	= of_match_ptr("vqmmc-r9a09g057-regulator"),
> +	.owner		= THIS_MODULE,
> +	.type		= REGULATOR_VOLTAGE,
> +	.ops		= &r9a09g057_regulator_voltage_ops,
> +	.volt_table	= r9a09g057_vqmmc_voltages,
> +	.n_voltages	= ARRAY_SIZE(r9a09g057_vqmmc_voltages),
> +};
> +
> +static const struct renesas_sdhi_of_data of_data_r9a09g057 = {
> +	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
> +			  TMIO_MMC_HAVE_CBSY | TMIO_MMC_MIN_RCAR2,
> +	.capabilities	= MMC_CAP_SD_HIGHSPEED | MMC_CAP_SDIO_IRQ |
> +			  MMC_CAP_CMD23 | MMC_CAP_WAIT_WHILE_BUSY,
> +	.capabilities2	= MMC_CAP2_NO_WRITE_PROTECT | MMC_CAP2_MERGE_CAPABLE,
> +	.bus_shift	= 2,
> +	.scc_offset	= 0x1000,
> +	.taps		= rcar_gen3_scc_taps,
> +	.taps_num	= ARRAY_SIZE(rcar_gen3_scc_taps),
> +	/* DMAC can handle 32bit blk count but only 1 segment */
> +	.max_blk_count	= UINT_MAX / TMIO_MAX_BLK_SIZE,
> +	.max_segs	= 1,
> +	.sdhi_flags	= SDHI_FLAG_NEED_CLKH_FALLBACK,
> +	.rdesc = &r9a09g057_vqmmc_regulator,
> +	.reg_init_data = &r9a09g057_regulator_init_data,
> +	.regulator_init_state = false,
> +	.regulator_init_voltage = 3300000,
> +	.regulator_force_endis = r9a09g057_regulator_force_endis,
> +	.regulator_force_voltage = r9a09g057_regulator_force_voltage,
> +};
> +
>  static const struct renesas_sdhi_of_data of_data_rza2 = {
>  	.tmio_flags	= TMIO_MMC_HAS_IDLE_WAIT | TMIO_MMC_CLK_ACTUAL |
>  			  TMIO_MMC_HAVE_CBSY,
> @@ -260,6 +404,11 @@ static const struct renesas_sdhi_of_data_with_quirks of_rzg2l_compatible = {
>  	.quirks = &sdhi_quirks_rzg2l,
>  };
>  
> +static const struct renesas_sdhi_of_data_with_quirks of_r9a09g057_compatible = {
> +	.of_data = &of_data_r9a09g057,
> +	.quirks = &sdhi_quirks_rzg2l,
> +};
> +
>  static const struct renesas_sdhi_of_data_with_quirks of_rcar_gen3_compatible = {
>  	.of_data = &of_data_rcar_gen3,
>  };
> @@ -284,6 +433,7 @@ static const struct of_device_id renesas_sdhi_internal_dmac_of_match[] = {
>  	{ .compatible = "renesas,sdhi-r8a77990", .data = &of_r8a77990_compatible, },
>  	{ .compatible = "renesas,sdhi-r8a77995", .data = &of_rcar_gen3_nohs400_compatible, },
>  	{ .compatible = "renesas,sdhi-r9a09g011", .data = &of_rzg2l_compatible, },
> +	{ .compatible = "renesas,sdhi-r9a09g057", .data = &of_r9a09g057_compatible, },
>  	{ .compatible = "renesas,rzg2l-sdhi", .data = &of_rzg2l_compatible, },
>  	{ .compatible = "renesas,rcar-gen3-sdhi", .data = &of_rcar_gen3_compatible, },
>  	{ .compatible = "renesas,rcar-gen4-sdhi", .data = &of_rcar_gen3_compatible, },
> diff --git a/drivers/mmc/host/tmio_mmc.h b/drivers/mmc/host/tmio_mmc.h
> index de56e6534aea..1a3172786662 100644
> --- a/drivers/mmc/host/tmio_mmc.h
> +++ b/drivers/mmc/host/tmio_mmc.h
> @@ -43,6 +43,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 and RZ/V2H(P) */
>  
>  /* Definitions for values the CTL_STOP_INTERNAL_ACTION register can take */
>  #define TMIO_STOP_STP		BIT(0)
> @@ -102,6 +103,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/V2H(P) */
> +#define SD_STATUS_IOVS		BIT(16) /* only known on RZ/V2H(P) */
> +
>  /* Define some IRQ masks */
>  /* This is the mask used at reset by the chip */
>  #define TMIO_MASK_ALL           0x837f031d