| Message ID | 20240624153229.68882-2-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add SD/MMC support for Renesas RZ/V2H(P) SoC | expand |
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 >
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
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
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
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
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
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
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
> -----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
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
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