| Message ID | 20241126114940.421143-2-ciprianmarian.costea@oss.nxp.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | add NXP RTC driver support for S32G2/S32G3 SoCs | expand |
On 26/11/2024 12:49, Ciprian Costea wrote: > + > +title: NXP S32G2/S32G3 Real Time Clock (RTC) > + > +maintainers: > + - Bogdan Hamciuc <bogdan.hamciuc@nxp.com> > + - Ciprian Marian Costea <ciprianmarian.costea@nxp.com> > + > +description: > + RTC hardware module present on S32G2/S32G3 SoCs is used as a wakeup source. > + It is not kept alive during system reset and it is not battery-powered. Does this mean that this is not a standard RTC thus standard RTC schema does not apply? > + > +properties: > + compatible: > + oneOf: > + - enum: > + - nxp,s32g2-rtc > + - items: > + - const: nxp,s32g3-rtc > + - const: nxp,s32g2-rtc > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + items: > + - description: ipg clock drives the access to the RTC iomapped registers > + - description: Clock source for the RTC module. Can be selected between > + 4 different clock sources using an integrated hardware mux. > + On S32G2/S32G3 SoCs, 'source0' is the SIRC clock (~32KHz) and it is > + available during standby and runtime. 'source1' is reserved and cannot I am not sure what are the benefits of allowing to choose a clock which cannot be used. I think source1 should be dropped. > + be used. 'source2' is the FIRC clock and it is only available during > + runtime providing a better resolution (~48MHz). 'source3' is an external > + RTC clock source which can be additionally added in hardware. > + > + clock-names: > + items: > + - const: ipg > + - enum: [ source0, source1, source2, source3 ] > + Best regards, Krzysztof
On 11/26/2024 9:08 PM, Krzysztof Kozlowski wrote: > On 26/11/2024 12:49, Ciprian Costea wrote: >> + >> +title: NXP S32G2/S32G3 Real Time Clock (RTC) >> + >> +maintainers: >> + - Bogdan Hamciuc <bogdan.hamciuc@nxp.com> >> + - Ciprian Marian Costea <ciprianmarian.costea@nxp.com> >> + >> +description: >> + RTC hardware module present on S32G2/S32G3 SoCs is used as a wakeup source. >> + It is not kept alive during system reset and it is not battery-powered. > > Does this mean that this is not a standard RTC thus standard RTC schema > does not apply? > Hello Krzysztof, I would say the standard RTC schema does apply but indeed you bring up a valid point in the fact that I forgot to reference 'rtc.yaml' schema. I will fix this in V6, by adding: allOf: - $ref: rtc.yaml# >> + >> +properties: >> + compatible: >> + oneOf: >> + - enum: >> + - nxp,s32g2-rtc >> + - items: >> + - const: nxp,s32g3-rtc >> + - const: nxp,s32g2-rtc >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + clocks: >> + items: >> + - description: ipg clock drives the access to the RTC iomapped registers >> + - description: Clock source for the RTC module. Can be selected between >> + 4 different clock sources using an integrated hardware mux. >> + On S32G2/S32G3 SoCs, 'source0' is the SIRC clock (~32KHz) and it is >> + available during standby and runtime. 'source1' is reserved and cannot > > I am not sure what are the benefits of allowing to choose a clock which > cannot be used. I think source1 should be dropped. > The current RTC support targets S32G2/S32G3 SoCs where 'source1' clock source cannot be used. The reasoning for allowing to choose it is that on future SoCs from S32 family the same RTC module may be integrated and 'source1' may become available. >> + be used. 'source2' is the FIRC clock and it is only available during >> + runtime providing a better resolution (~48MHz). 'source3' is an external >> + RTC clock source which can be additionally added in hardware. >> + >> + clock-names: >> + items: >> + - const: ipg >> + - enum: [ source0, source1, source2, source3 ] >> + > Best regards, > Krzysztof Best Regards, Ciprian
On Tue, Nov 26, 2024 at 01:49:37PM +0200, Ciprian Costea wrote: > From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com> > > RTC tracks clock time during system suspend and it is used as a wakeup > source on S32G2/S32G3 architecture. > > RTC from S32G2/S32G3 is not battery-powered and it is not kept alive > during system reset. > > Co-developed-by: Bogdan-Gabriel Roman <bogdan-gabriel.roman@nxp.com> > Signed-off-by: Bogdan-Gabriel Roman <bogdan-gabriel.roman@nxp.com> > Co-developed-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> > Signed-off-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> > Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com> > --- > .../devicetree/bindings/rtc/nxp,s32g-rtc.yaml | 69 +++++++++++++++++++ > 1 file changed, 69 insertions(+) > create mode 100644 Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml > > diff --git a/Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml b/Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml > new file mode 100644 > index 000000000000..89414a0d926c > --- /dev/null > +++ b/Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml > @@ -0,0 +1,69 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/rtc/nxp,s32g-rtc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: NXP S32G2/S32G3 Real Time Clock (RTC) > + > +maintainers: > + - Bogdan Hamciuc <bogdan.hamciuc@nxp.com> > + - Ciprian Marian Costea <ciprianmarian.costea@nxp.com> > + > +description: > + RTC hardware module present on S32G2/S32G3 SoCs is used as a wakeup source. > + It is not kept alive during system reset and it is not battery-powered. > + > +properties: > + compatible: > + oneOf: > + - enum: > + - nxp,s32g2-rtc > + - items: > + - const: nxp,s32g3-rtc > + - const: nxp,s32g2-rtc > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + items: > + - description: ipg clock drives the access to the RTC iomapped registers > + - description: Clock source for the RTC module. Can be selected between > + 4 different clock sources using an integrated hardware mux. > + On S32G2/S32G3 SoCs, 'source0' is the SIRC clock (~32KHz) and it is > + available during standby and runtime. 'source1' is reserved and cannot > + be used. 'source2' is the FIRC clock and it is only available during > + runtime providing a better resolution (~48MHz). 'source3' is an external > + RTC clock source which can be additionally added in hardware. Is switching the clock source at run-time possible? For example, use the 48MHz at runtime and switch to 32kHz or external clock during suspend. If so, you need to list all possible clock sources. Really, you probably should no matter what as you need to describe what's in the h/w, not configuration (though configuration is okay when it's fixed for the device). > + > + clock-names: > + items: > + - const: ipg > + - enum: [ source0, source1, source2, source3 ] You can do: maxItems: 5 items: - const: ipg additionalItems: pattern: '^source[0-4]$' Though I will have to relax constraints on 'additionalItems' to avoid a warning. Rob
On 11/27/2024 4:43 PM, Rob Herring wrote: > On Tue, Nov 26, 2024 at 01:49:37PM +0200, Ciprian Costea wrote: >> From: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com> >> >> RTC tracks clock time during system suspend and it is used as a wakeup >> source on S32G2/S32G3 architecture. >> >> RTC from S32G2/S32G3 is not battery-powered and it is not kept alive >> during system reset. >> >> Co-developed-by: Bogdan-Gabriel Roman <bogdan-gabriel.roman@nxp.com> >> Signed-off-by: Bogdan-Gabriel Roman <bogdan-gabriel.roman@nxp.com> >> Co-developed-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> >> Signed-off-by: Ghennadi Procopciuc <ghennadi.procopciuc@nxp.com> >> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea@oss.nxp.com> >> --- >> .../devicetree/bindings/rtc/nxp,s32g-rtc.yaml | 69 +++++++++++++++++++ >> 1 file changed, 69 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml >> >> diff --git a/Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml b/Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml >> new file mode 100644 >> index 000000000000..89414a0d926c >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml >> @@ -0,0 +1,69 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/rtc/nxp,s32g-rtc.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: NXP S32G2/S32G3 Real Time Clock (RTC) >> + >> +maintainers: >> + - Bogdan Hamciuc <bogdan.hamciuc@nxp.com> >> + - Ciprian Marian Costea <ciprianmarian.costea@nxp.com> >> + >> +description: >> + RTC hardware module present on S32G2/S32G3 SoCs is used as a wakeup source. >> + It is not kept alive during system reset and it is not battery-powered. >> + >> +properties: >> + compatible: >> + oneOf: >> + - enum: >> + - nxp,s32g2-rtc >> + - items: >> + - const: nxp,s32g3-rtc >> + - const: nxp,s32g2-rtc >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + clocks: >> + items: >> + - description: ipg clock drives the access to the RTC iomapped registers >> + - description: Clock source for the RTC module. Can be selected between >> + 4 different clock sources using an integrated hardware mux. >> + On S32G2/S32G3 SoCs, 'source0' is the SIRC clock (~32KHz) and it is >> + available during standby and runtime. 'source1' is reserved and cannot >> + be used. 'source2' is the FIRC clock and it is only available during >> + runtime providing a better resolution (~48MHz). 'source3' is an external >> + RTC clock source which can be additionally added in hardware. > > Is switching the clock source at run-time possible? For example, use the > 48MHz at runtime and switch to 32kHz or external clock during suspend. > If so, you need to list all possible clock sources. Really, you probably > should no matter what as you need to describe what's in the h/w, not > configuration (though configuration is okay when it's fixed for the > device). > Hello Rob, Thank you for your review. In this latest V5 of this patchset, clock source switching at run-time/suspend support has been dropped (as agreed during the review process). Therefore a static clock source configuration is used for a specific S32G SoC which uses the RTC device. >> + >> + clock-names: >> + items: >> + - const: ipg >> + - enum: [ source0, source1, source2, source3 ] > > You can do: > > maxItems: 5 > items: > - const: ipg > additionalItems: > pattern: '^source[0-4]$' > > Though I will have to relax constraints on 'additionalItems' to avoid a > warning. > > Rob Thanks for your suggestion. I will consider it for V6. Best Regards, Ciprian
diff --git a/Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml b/Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml new file mode 100644 index 000000000000..89414a0d926c --- /dev/null +++ b/Documentation/devicetree/bindings/rtc/nxp,s32g-rtc.yaml @@ -0,0 +1,69 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/rtc/nxp,s32g-rtc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NXP S32G2/S32G3 Real Time Clock (RTC) + +maintainers: + - Bogdan Hamciuc <bogdan.hamciuc@nxp.com> + - Ciprian Marian Costea <ciprianmarian.costea@nxp.com> + +description: + RTC hardware module present on S32G2/S32G3 SoCs is used as a wakeup source. + It is not kept alive during system reset and it is not battery-powered. + +properties: + compatible: + oneOf: + - enum: + - nxp,s32g2-rtc + - items: + - const: nxp,s32g3-rtc + - const: nxp,s32g2-rtc + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clocks: + items: + - description: ipg clock drives the access to the RTC iomapped registers + - description: Clock source for the RTC module. Can be selected between + 4 different clock sources using an integrated hardware mux. + On S32G2/S32G3 SoCs, 'source0' is the SIRC clock (~32KHz) and it is + available during standby and runtime. 'source1' is reserved and cannot + be used. 'source2' is the FIRC clock and it is only available during + runtime providing a better resolution (~48MHz). 'source3' is an external + RTC clock source which can be additionally added in hardware. + + clock-names: + items: + - const: ipg + - enum: [ source0, source1, source2, source3 ] + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/interrupt-controller/irq.h> + + rtc@40060000 { + compatible = "nxp,s32g3-rtc", + "nxp,s32g2-rtc"; + reg = <0x40060000 0x1000>; + interrupts = <GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clks 54>, <&clks 55>; + clock-names = "ipg", "source0"; + };