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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
 <TY3PR01MB11346179D6B17BE023E6C33E886D52@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Tue, 25 Jun 2024 09:47:44 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
CC: Ulf Hansson <ulf.hansson@...aro.org>, Wolfram Sang
	<wsa+renesas@...g-engineering.com>, Geert Uytterhoeven
	<geert+renesas@...der.be>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski
	<krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Magnus Damm
	<magnus.damm@...il.com>, "linux-mmc@...r.kernel.org"
	<linux-mmc@...r.kernel.org>, "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-renesas-soc@...r.kernel.org"
	<linux-renesas-soc@...r.kernel.org>, Fabrizio Castro
	<fabrizio.castro.jz@...esas.com>, Prabhakar Mahadev Lad
	<prabhakar.mahadev-lad.rj@...renesas.com>
Subject: RE: [PATCH v3 1/3] dt-bindings: mmc: renesas,sdhi: Document RZ/V2H(P)
 support



> -----Original Message-----
> From: Lad, Prabhakar <prabhakar.csengg@...il.com>
> Sent: Tuesday, June 25, 2024 10:19 AM
> To: Biju Das <biju.das.jz@...renesas.com>
> Cc: Ulf Hansson <ulf.hansson@...aro.org>; Wolfram Sang <wsa+renesas@...g-engineering.com>; Geert
> Uytterhoeven <geert+renesas@...der.be>; Rob Herring <robh@...nel.org>; Krzysztof Kozlowski
> <krzk+dt@...nel.org>; Conor Dooley <conor+dt@...nel.org>; Magnus Damm <magnus.damm@...il.com>;
> linux-mmc@...r.kernel.org; devicetree@...r.kernel.org; linux-kernel@...r.kernel.org; linux-renesas-
> soc@...r.kernel.org; Fabrizio Castro <fabrizio.castro.jz@...esas.com>; Prabhakar Mahadev Lad
> <prabhakar.mahadev-lad.rj@...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@...renesas.com> wrote:
> >
> > Hi Prabhakar,
> >
> > > -----Original Message-----
> > > From: Prabhakar <prabhakar.csengg@...il.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@...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@...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