diff mbox series

[1/3] dt-bindings: mfd: Document the RTC present on MAX77620

Message ID 20200417170825.2551367-1-thierry.reding@gmail.com
State New
Headers show
Series [1/3] dt-bindings: mfd: Document the RTC present on MAX77620 | expand

Commit Message

Thierry Reding April 17, 2020, 5:08 p.m. UTC
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(+)

Comments

Rob Herring April 30, 2020, 2:07 p.m. UTC | #1
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
Alexandre Belloni April 30, 2020, 2:15 p.m. UTC | #2
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.
Rob Herring May 1, 2020, 1 p.m. UTC | #3
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
Alexandre Belloni May 1, 2020, 1:53 p.m. UTC | #4
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.
Thierry Reding May 8, 2020, 11:02 a.m. UTC | #5
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
Alexandre Belloni May 11, 2020, 2:28 p.m. UTC | #6
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 mbox series

Patch

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;
+	};
 };