[v4,09/11] regulator: dt-bindings: Add Renesas RAA215300 PMIC bindings

Document Renesas RAA215300 PMIC bindings.

The RAA215300 is a high Performance 9-Channel PMIC supporting DDR
Memory, with Built-In Charger and RTC.

It supports DDR3, DDR3L, DDR4, and LPDDR4 memory power requirements.
The internally compensated regulators, built-in Real-Time Clock (RTC),
32kHz crystal oscillator, and coin cell battery charger provide a
highly integrated, small footprint power solution ideal for
System-On-Module (SOM) applications. A spread spectrum feature
provides an ease-of-use solution for noise-sensitive audio or RF
applications.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v3->v4:
 * Moved bindings from mfd->regulator.
 * Dropped minItems from reg.
 * Dropped renesas,rtc-enabled property and instead used clock-names property
   to find RTC is enabled or not.
 * Added reg-names in required property.
 * Updated the example.
v2->v3:
 * Added more detailed description for renesas,rtc-enabled property.
RFC->v2:
 * Updated reg property
 * Added optional reg-names, interrupts and renesas,rtc-enabled
   properties.
 * Fixed the node name in the example
 * Dropped the cross link property renesas,raa215300-rtc.
 * Updated the example
---
 .../bindings/regulator/renesas,raa215300.yaml | 84 +++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/renesas,raa215300.yaml

Hi Conor,

Thanks for the feedback.

> -----Original Message-----
> From: Conor Dooley <conor.dooley@microchip.com>
> Sent: Friday, May 19, 2023 3:10 PM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Conor Dooley <conor@kernel.org>; Rob Herring <robh+dt@kernel.org>;
> Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Liam Girdwood <lgirdwood@gmail.com>; Mark Brown
> <broonie@kernel.org>; Geert Uytterhoeven <geert+renesas@glider.be>;
> Magnus Damm <magnus.damm@gmail.com>; devicetree@vger.kernel.org; linux-
> renesas-soc@vger.kernel.org; Fabrizio Castro
> <fabrizio.castro.jz@renesas.com>
> Subject: Re: [PATCH v4 09/11] regulator: dt-bindings: Add Renesas
> RAA215300 PMIC bindings
> 
> On Fri, May 19, 2023 at 06:53:03AM +0000, Biju Das wrote:
> > > Subject: Re: [PATCH v4 09/11] regulator: dt-bindings: Add Renesas
> > > RAA215300 PMIC bindings
> > >
> > > On Thu, May 18, 2023 at 12:36:41PM +0100, Biju Das wrote:
> > > > Document Renesas RAA215300 PMIC bindings.
> > > >
> > > > The RAA215300 is a high Performance 9-Channel PMIC supporting DDR
> > > > Memory, with Built-In Charger and RTC.
> > > >
> > > > It supports DDR3, DDR3L, DDR4, and LPDDR4 memory power
> requirements.
> > > > The internally compensated regulators, built-in Real-Time Clock
> > > > (RTC), 32kHz crystal oscillator, and coin cell battery charger
> > > > provide a highly integrated, small footprint power solution ideal
> > > > for System-On-Module (SOM) applications. A spread spectrum feature
> > > > provides an ease-of-use solution for noise-sensitive audio or RF
> > > > applications.
> > > > +  reg:
> > > > +    maxItems: 2
> > > > +
> > > > +  reg-names:
> > > > +    items:
> > > > +      - const: main
> > > > +      - const: rtc
> > >
> > > > +required:
> > > > +  - compatible
> > > > +  - reg
> > > > +  - reg-names
> > >
> > > Out of curiosity as much as anything else, why do you need reg-names
> > > if the two registers are always required?
> >
> > The device has always 2 address spaces and "reg-names" provides a
> > means of clear differentiation compared to indices.
> >
> > By enforcing "reg-names" as required property, dt can do some
> > schema-validation forcing users to specify "reg-names" in device tree.
> 
> Is that not what we have the following for:
>   reg:
>    items:
>      - description: main register space...
>      - description: special sauce rtc stuff...
> ?

OK, will add description.

> 
> The schema validation doesn't stop them putting in the wrong address
> either way!

OK, will drop reg-names from required property.

> 
> > Implementation wise, we use "rtc" for getting resource details while
> > creating the second i2c device which overrides the default address.
> >
> > Strictly speaking reg-names is not required, but from a validation
> > perspective and since driver is using the same "resource-name" it is
> > better to have it??
> 
> If the order is set by the descriptions, reg-names seem superfluous
> /shrug

Agreed.

Cheers,
Biju

Hi Biju,

On Fri, May 19, 2023 at 4:39 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > -----Original Message-----
> > From: Conor Dooley <conor.dooley@microchip.com>
> > Sent: Friday, May 19, 2023 3:10 PM
> > To: Biju Das <biju.das.jz@bp.renesas.com>
> > Cc: Conor Dooley <conor@kernel.org>; Rob Herring <robh+dt@kernel.org>;
> > Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> > <conor+dt@kernel.org>; Liam Girdwood <lgirdwood@gmail.com>; Mark Brown
> > <broonie@kernel.org>; Geert Uytterhoeven <geert+renesas@glider.be>;
> > Magnus Damm <magnus.damm@gmail.com>; devicetree@vger.kernel.org; linux-
> > renesas-soc@vger.kernel.org; Fabrizio Castro
> > <fabrizio.castro.jz@renesas.com>
> > Subject: Re: [PATCH v4 09/11] regulator: dt-bindings: Add Renesas
> > RAA215300 PMIC bindings
> >
> > On Fri, May 19, 2023 at 06:53:03AM +0000, Biju Das wrote:
> > > > Subject: Re: [PATCH v4 09/11] regulator: dt-bindings: Add Renesas
> > > > RAA215300 PMIC bindings
> > > >
> > > > On Thu, May 18, 2023 at 12:36:41PM +0100, Biju Das wrote:
> > > > > Document Renesas RAA215300 PMIC bindings.
> > > > >
> > > > > The RAA215300 is a high Performance 9-Channel PMIC supporting DDR
> > > > > Memory, with Built-In Charger and RTC.
> > > > >
> > > > > It supports DDR3, DDR3L, DDR4, and LPDDR4 memory power
> > requirements.
> > > > > The internally compensated regulators, built-in Real-Time Clock
> > > > > (RTC), 32kHz crystal oscillator, and coin cell battery charger
> > > > > provide a highly integrated, small footprint power solution ideal
> > > > > for System-On-Module (SOM) applications. A spread spectrum feature
> > > > > provides an ease-of-use solution for noise-sensitive audio or RF
> > > > > applications.
> > > > > +  reg:
> > > > > +    maxItems: 2
> > > > > +
> > > > > +  reg-names:
> > > > > +    items:
> > > > > +      - const: main
> > > > > +      - const: rtc
> > > >
> > > > > +required:
> > > > > +  - compatible
> > > > > +  - reg
> > > > > +  - reg-names
> > > >
> > > > Out of curiosity as much as anything else, why do you need reg-names
> > > > if the two registers are always required?
> > >
> > > The device has always 2 address spaces and "reg-names" provides a
> > > means of clear differentiation compared to indices.
> > >
> > > By enforcing "reg-names" as required property, dt can do some
> > > schema-validation forcing users to specify "reg-names" in device tree.
> >
> > Is that not what we have the following for:
> >   reg:
> >    items:
> >      - description: main register space...
> >      - description: special sauce rtc stuff...
> > ?
>
> OK, will add description.
>
> >
> > The schema validation doesn't stop them putting in the wrong address
> > either way!
>
> OK, will drop reg-names from required property.

Please don't, as i2c_new_ancillary_device() does rely on the name
to set the address when overriding from the default.

> > > Implementation wise, we use "rtc" for getting resource details while
> > > creating the second i2c device which overrides the default address.
> > >
> > > Strictly speaking reg-names is not required, but from a validation
> > > perspective and since driver is using the same "resource-name" it is
> > > better to have it??
> >
> > If the order is set by the descriptions, reg-names seem superfluous
> > /shrug

We have plenty of these, as it turned out to be much easier for validation
to fix the order in the bindings than to support random order ;-)
Lots of Linux drivers that do use the names don't care about the order.
If all actual DTS used the same order, usually the order was fixed during
the text to json-schema conversion...

Gr{oetje,eeting}s,

                        Geert

On Fri, May 19, 2023 at 03:10:25PM +0100, Conor Dooley wrote:
> On Fri, May 19, 2023 at 06:53:03AM +0000, Biju Das wrote:

> > By enforcing "reg-names" as required property, dt can do some schema-validation
> > forcing users to specify "reg-names" in device tree.

> Is that not what we have the following for:
>   reg:
>    items:
>      - description: main register space...
>      - description: special sauce rtc stuff...
> ?

> The schema validation doesn't stop them putting in the wrong address
> either way!

Being able to look things up by name does help make the code using the
binding more readable, and it helps with reading the DTs too.

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH v4 09/11] regulator: dt-bindings: Add Renesas
> RAA215300 PMIC bindings
> 
> Hi Biju,
> 
> On Thu, May 18, 2023 at 1:37 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Document Renesas RAA215300 PMIC bindings.
> >
> > The RAA215300 is a high Performance 9-Channel PMIC supporting DDR
> > Memory, with Built-In Charger and RTC.
> >
> > It supports DDR3, DDR3L, DDR4, and LPDDR4 memory power requirements.
> > The internally compensated regulators, built-in Real-Time Clock (RTC),
> > 32kHz crystal oscillator, and coin cell battery charger provide a
> > highly integrated, small footprint power solution ideal for
> > System-On-Module (SOM) applications. A spread spectrum feature
> > provides an ease-of-use solution for noise-sensitive audio or RF
> > applications.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v3->v4:
> >  * Moved bindings from mfd->regulator.
> >  * Dropped minItems from reg.
> >  * Dropped renesas,rtc-enabled property and instead used clock-names
> property
> >    to find RTC is enabled or not.
> >  * Added reg-names in required property.
> >  * Updated the example.
> 
> Thanks for your patch!
> 
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/regulator/renesas,raa215300.ya
> > +++ ml
> > @@ -0,0 +1,84 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause %YAML 1.2
> > +---
> > +$id:
> > +title: Renesas RAA215300 Power Management Integrated Circuit (PMIC)
> > +
> > +maintainers:
> > +  - Biju Das <biju.das.jz@bp.renesas.com>
> > +
> > +description: |
> > +  The RAA215300 is a high-performance, low-cost 9-channel PMIC
> > +designed for
> > +  32-bit and 64-bit MCU and MPU applications. It supports DDR3,
> > +DDR3L, DDR4,
> > +  and LPDDR4 memory power requirements. The internally compensated
> > +regulators,
> > +  built-in Real-Time Clock (RTC), 32kHz crystal oscillator, and coin
> > +cell
> > +  battery charger provide a highly integrated, small footprint power
> > +solution
> > +  ideal for System-On-Module (SOM) applications. A spread spectrum
> > +feature
> > +  provides an ease-of-use solution for noise-sensitive audio or RF
> applications.
> > +
> > +  This device exposes two devices via I2C. One for the integrated RTC
> > + IP, and  one for everything else.
> > +
> > +  Link to datasheet:
> > +
> > + https://www.renesas.com/in/en/products/power-power-management/multi-
> > + channel-power-management-ics-pmics/ssdsoc-power-management-ics-pmic-
> > + and-pmus/raa215300-high-performance-9-channel-pmic-supporting-ddr-me
> > + mory-built-charger-and-rtc
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - renesas,raa215300
> > +
> > +  reg:
> > +    maxItems: 2
> > +
> > +  reg-names:
> > +    items:
> > +      - const: main
> > +      - const: rtc
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> 
> Perhaps add a description to make it clear that not providing clocks is
> supported, and means the RTC is disabled?

Agreed.

> 
> > +
> > +  clock-names:
> > +    description: |
> > +      Use xin, if connected to an external crystal.
> > +      Use clkin, if connected to an external clock signal.
> > +    oneOf:
> > +      - items:
> > +          - const: xin
> > +      - items:
> > +          - const: clkin
> 
> Please replace oneOf+items by enum.

OK.

Cheers,
Biju

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names