| Message ID | 20200417170825.2551367-1-thierry.reding@gmail.com |
|---|---|
| State | Rejected |
| Headers | show |
| Series | [1/3] dt-bindings: mfd: Document the RTC present on MAX77620 | expand |
On Fri, Apr 17, 2020 at 07:08:23PM +0200, Thierry Reding wrote: > From: Thierry Reding <treding@nvidia.com> > > The RTC present on MAX77620 can be used to generate an alarm at a given > time, which in turn can be used as a wakeup source for the system if it > is properly wired up. > > Document how to enable the RTC to act as a wakeup source. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > --- > .../devicetree/bindings/mfd/max77620.txt | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt > index 5a642a51d58e..f05005b0993e 100644 > --- a/Documentation/devicetree/bindings/mfd/max77620.txt > +++ b/Documentation/devicetree/bindings/mfd/max77620.txt > @@ -125,6 +125,17 @@ MAX77663 supports 20, 40, 80, 160, 320, 640, 1280 and 2540 microseconds. > control) then, GPIO1/nRST_IO goes LOW. > this property is valid for max20024 only. > > +Realtime Clock > +-------------- > +The MAX77620 family of power management ICs contain a realtime clock block > +that can be used to keep track of time even when the system is powered off. > + > +The realtime clock can also be programmed to trigger alerts, which can be > +used to wake the system up from sleep. In order to configure the RTC to act > +as a wakeup source, add an "rtc" child node and add the "wakeup-source" > +property. > + > + > For DT binding details of different sub modules like GPIO, pincontrol, > regulator, power, please refer respective device-tree binding document > under their respective sub-system directories. > @@ -159,4 +170,8 @@ max77620@3c { > maxim,fps-event-source = <MAX77620_FPS_EVENT_SRC_SW>; > }; > }; > + > + rtc { > + wakeup-source; Is the RTC really the only thing that could wake the system in this PMIC? I don't think it's really valid to have 'wakeup-source' without 'interrupts' unless the wakeup mechanism is somehow not an interrupt. So I think this belongs in the parent node. Rob
On 30/04/2020 09:07:01-0500, Rob Herring wrote: > On Fri, Apr 17, 2020 at 07:08:23PM +0200, Thierry Reding wrote: > > From: Thierry Reding <treding@nvidia.com> > > > > The RTC present on MAX77620 can be used to generate an alarm at a given > > time, which in turn can be used as a wakeup source for the system if it > > is properly wired up. > > > > Document how to enable the RTC to act as a wakeup source. > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > --- > > .../devicetree/bindings/mfd/max77620.txt | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt > > index 5a642a51d58e..f05005b0993e 100644 > > --- a/Documentation/devicetree/bindings/mfd/max77620.txt > > +++ b/Documentation/devicetree/bindings/mfd/max77620.txt > > @@ -125,6 +125,17 @@ MAX77663 supports 20, 40, 80, 160, 320, 640, 1280 and 2540 microseconds. > > control) then, GPIO1/nRST_IO goes LOW. > > this property is valid for max20024 only. > > > > +Realtime Clock > > +-------------- > > +The MAX77620 family of power management ICs contain a realtime clock block > > +that can be used to keep track of time even when the system is powered off. > > + > > +The realtime clock can also be programmed to trigger alerts, which can be > > +used to wake the system up from sleep. In order to configure the RTC to act > > +as a wakeup source, add an "rtc" child node and add the "wakeup-source" > > +property. > > + > > + > > For DT binding details of different sub modules like GPIO, pincontrol, > > regulator, power, please refer respective device-tree binding document > > under their respective sub-system directories. > > @@ -159,4 +170,8 @@ max77620@3c { > > maxim,fps-event-source = <MAX77620_FPS_EVENT_SRC_SW>; > > }; > > }; > > + > > + rtc { > > + wakeup-source; > > Is the RTC really the only thing that could wake the system in this > PMIC? > > I don't think it's really valid to have 'wakeup-source' without > 'interrupts' unless the wakeup mechanism is somehow not an interrupt. So > I think this belongs in the parent node. > I don't think this is true because in the case of a discrete RTC, its interrupt pin can be connected directly to a PMIC to power up a board instead of being connected to the SoC. In that case we don't have an interrupt property but the RTC is still a wakeup source. This is the usual use case for wakeup-source in the RTC subsystem. Else, if there is an interrupt, then we assume the RTC is a wakeup source and there is no need to have the wakeup-source property.
On Thu, Apr 30, 2020 at 9:15 AM Alexandre Belloni <alexandre.belloni@bootlin.com> wrote: > > On 30/04/2020 09:07:01-0500, Rob Herring wrote: > > On Fri, Apr 17, 2020 at 07:08:23PM +0200, Thierry Reding wrote: > > > From: Thierry Reding <treding@nvidia.com> > > > > > > The RTC present on MAX77620 can be used to generate an alarm at a given > > > time, which in turn can be used as a wakeup source for the system if it > > > is properly wired up. > > > > > > Document how to enable the RTC to act as a wakeup source. > > > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > > --- > > > .../devicetree/bindings/mfd/max77620.txt | 15 +++++++++++++++ > > > 1 file changed, 15 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt > > > index 5a642a51d58e..f05005b0993e 100644 > > > --- a/Documentation/devicetree/bindings/mfd/max77620.txt > > > +++ b/Documentation/devicetree/bindings/mfd/max77620.txt > > > @@ -125,6 +125,17 @@ MAX77663 supports 20, 40, 80, 160, 320, 640, 1280 and 2540 microseconds. > > > control) then, GPIO1/nRST_IO goes LOW. > > > this property is valid for max20024 only. > > > > > > +Realtime Clock > > > +-------------- > > > +The MAX77620 family of power management ICs contain a realtime clock block > > > +that can be used to keep track of time even when the system is powered off. > > > + > > > +The realtime clock can also be programmed to trigger alerts, which can be > > > +used to wake the system up from sleep. In order to configure the RTC to act > > > +as a wakeup source, add an "rtc" child node and add the "wakeup-source" > > > +property. > > > + > > > + > > > For DT binding details of different sub modules like GPIO, pincontrol, > > > regulator, power, please refer respective device-tree binding document > > > under their respective sub-system directories. > > > @@ -159,4 +170,8 @@ max77620@3c { > > > maxim,fps-event-source = <MAX77620_FPS_EVENT_SRC_SW>; > > > }; > > > }; > > > + > > > + rtc { > > > + wakeup-source; > > > > Is the RTC really the only thing that could wake the system in this > > PMIC? > > > > I don't think it's really valid to have 'wakeup-source' without > > 'interrupts' unless the wakeup mechanism is somehow not an interrupt. So > > I think this belongs in the parent node. > > > > I don't think this is true because in the case of a discrete RTC, its > interrupt pin can be connected directly to a PMIC to power up a board > instead of being connected to the SoC. In that case we don't have an > interrupt property but the RTC is still a wakeup source. This is the > usual use case for wakeup-source in the RTC subsystem. Else, if there is > an interrupt, then we assume the RTC is a wakeup source and there is no > need to have the wakeup-source property. Yes, that would be an example of "unless the wakeup mechanism is somehow not an interrupt". I guess I should add not an interrupt from the perspective of the OS. So if the wakeup is self contained within the PMIC, why do we need a DT property? The capability is always there and enabling/disabling wakeup from it is userspace policy. Rob
On 01/05/2020 08:00:11-0500, Rob Herring wrote: > > I don't think this is true because in the case of a discrete RTC, its > > interrupt pin can be connected directly to a PMIC to power up a board > > instead of being connected to the SoC. In that case we don't have an > > interrupt property but the RTC is still a wakeup source. This is the > > usual use case for wakeup-source in the RTC subsystem. Else, if there is > > an interrupt, then we assume the RTC is a wakeup source and there is no > > need to have the wakeup-source property. > > Yes, that would be an example of "unless the wakeup mechanism is > somehow not an interrupt". I guess I should add not an interrupt from > the perspective of the OS. > > So if the wakeup is self contained within the PMIC, why do we need a > DT property? The capability is always there and enabling/disabling > wakeup from it is userspace policy. > Yes, for this particular case, I'm not sure wakeup-source is actually necessary. If the interrupt line is used to wakeup the SoC, then the presence of the interrupts property is enough to enable wakeup.
On Fri, May 01, 2020 at 03:53:09PM +0200, Alexandre Belloni wrote: > On 01/05/2020 08:00:11-0500, Rob Herring wrote: > > > I don't think this is true because in the case of a discrete RTC, its > > > interrupt pin can be connected directly to a PMIC to power up a board > > > instead of being connected to the SoC. In that case we don't have an > > > interrupt property but the RTC is still a wakeup source. This is the > > > usual use case for wakeup-source in the RTC subsystem. Else, if there is > > > an interrupt, then we assume the RTC is a wakeup source and there is no > > > need to have the wakeup-source property. > > > > Yes, that would be an example of "unless the wakeup mechanism is > > somehow not an interrupt". I guess I should add not an interrupt from > > the perspective of the OS. > > > > So if the wakeup is self contained within the PMIC, why do we need a > > DT property? The capability is always there and enabling/disabling > > wakeup from it is userspace policy. > > > > Yes, for this particular case, I'm not sure wakeup-source is actually > necessary. If the interrupt line is used to wakeup the SoC, then the > presence of the interrupts property is enough to enable wakeup. So yes, the wakeup-source property isn't necessary. The goal of patches 1 and 2 was to allow the RTC to be actually disabled as a wakeup-source in case it didn't work as intended. But since the RTC is enabled as a wakeup source on these PMICs by default, the idea was to add a new sub- node for the RTC and required the wakeup-source in that subnode if that subnode was present. That said, patch 3 actually does make the RTC work as a wakeup source on the particular board that I tested this, so patches 1 and 2 are no longer really required from my point of view. Do you want me to send patch 3/3 again separately or can you pick it up from this series? Thanks, Thierry
Hi, On 08/05/2020 13:02:26+0200, Thierry Reding wrote: > On Fri, May 01, 2020 at 03:53:09PM +0200, Alexandre Belloni wrote: > > On 01/05/2020 08:00:11-0500, Rob Herring wrote: > > > > I don't think this is true because in the case of a discrete RTC, its > > > > interrupt pin can be connected directly to a PMIC to power up a board > > > > instead of being connected to the SoC. In that case we don't have an > > > > interrupt property but the RTC is still a wakeup source. This is the > > > > usual use case for wakeup-source in the RTC subsystem. Else, if there is > > > > an interrupt, then we assume the RTC is a wakeup source and there is no > > > > need to have the wakeup-source property. > > > > > > Yes, that would be an example of "unless the wakeup mechanism is > > > somehow not an interrupt". I guess I should add not an interrupt from > > > the perspective of the OS. > > > > > > So if the wakeup is self contained within the PMIC, why do we need a > > > DT property? The capability is always there and enabling/disabling > > > wakeup from it is userspace policy. > > > > > > > Yes, for this particular case, I'm not sure wakeup-source is actually > > necessary. If the interrupt line is used to wakeup the SoC, then the > > presence of the interrupts property is enough to enable wakeup. > > So yes, the wakeup-source property isn't necessary. The goal of patches > 1 and 2 was to allow the RTC to be actually disabled as a wakeup-source > in case it didn't work as intended. But since the RTC is enabled as a > wakeup source on these PMICs by default, the idea was to add a new sub- > node for the RTC and required the wakeup-source in that subnode if that > subnode was present. > > That said, patch 3 actually does make the RTC work as a wakeup source > on the particular board that I tested this, so patches 1 and 2 are no > longer really required from my point of view. > > Do you want me to send patch 3/3 again separately or can you pick it up > from this series? > I applied it.
diff --git a/Documentation/devicetree/bindings/mfd/max77620.txt b/Documentation/devicetree/bindings/mfd/max77620.txt index 5a642a51d58e..f05005b0993e 100644 --- a/Documentation/devicetree/bindings/mfd/max77620.txt +++ b/Documentation/devicetree/bindings/mfd/max77620.txt @@ -125,6 +125,17 @@ MAX77663 supports 20, 40, 80, 160, 320, 640, 1280 and 2540 microseconds. control) then, GPIO1/nRST_IO goes LOW. this property is valid for max20024 only. +Realtime Clock +-------------- +The MAX77620 family of power management ICs contain a realtime clock block +that can be used to keep track of time even when the system is powered off. + +The realtime clock can also be programmed to trigger alerts, which can be +used to wake the system up from sleep. In order to configure the RTC to act +as a wakeup source, add an "rtc" child node and add the "wakeup-source" +property. + + For DT binding details of different sub modules like GPIO, pincontrol, regulator, power, please refer respective device-tree binding document under their respective sub-system directories. @@ -159,4 +170,8 @@ max77620@3c { maxim,fps-event-source = <MAX77620_FPS_EVENT_SRC_SW>; }; }; + + rtc { + wakeup-source; + }; };