| Message ID | 20230518113643.420806-10-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | Add Renesas PMIC RAA215300 and built-in RTC support | expand |
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? Cheers, Conor.
Hi Conor, Thanks for the feedback. > 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. 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?? Please share your thoughts. Cheers, Biju
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.yaml > @@ -0,0 +1,84 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/regulator/renesas,raa215300.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +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-memory-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? > + > + 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. > + > +required: > + - compatible > + - reg > + - reg-names Gr{oetje,eeting}s, Geert
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... ? The schema validation doesn't stop them putting in the wrong address either way! > 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 Cheers, Conor.
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 4:49 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Fri, May 19, 2023 at 4:39 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > From: Conor Dooley <conor.dooley@microchip.com> > > > 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 I meant "When all actual DTS...". > 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.
On Fri, May 19, 2023 at 04:49:44PM +0200, Geert Uytterhoeven wrote: > 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> > > > On Fri, May 19, 2023 at 06:53:03AM +0000, Biju Das wrote: > > > > > Subject: Re: [PATCH v4 09/11] regulator: dt-bindings: Add Renesas > > > > > > +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? > > 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. That looks like my answer! Thanks Geert & sorry for the noise here. Modulo Geert's requested change, I think I owe you a Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Biju. Thanks, Conor.
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
diff --git a/Documentation/devicetree/bindings/regulator/renesas,raa215300.yaml b/Documentation/devicetree/bindings/regulator/renesas,raa215300.yaml new file mode 100644 index 000000000000..17b16f66f695 --- /dev/null +++ b/Documentation/devicetree/bindings/regulator/renesas,raa215300.yaml @@ -0,0 +1,84 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/regulator/renesas,raa215300.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +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-memory-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 + + 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 + +required: + - compatible + - reg + - reg-names + +additionalProperties: false + +examples: + - | + /* 32.768kHz crystal */ + x2: clock-xtal { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <32768>; + }; + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + raa215300: pmic@12 { + compatible = "renesas,raa215300"; + reg = <0x12>, <0x6f>; + reg-names = "main", "rtc"; + + clocks = <&x2>; + clock-names = "xin"; + }; + };
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