From: Thierry Reding <[email protected]>
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 <[email protected]>
---
.../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;
+ };
};
--
2.24.1
From: Thierry Reding <[email protected]>
The MAX77620 doesn't support bulk writes, so make sure the regmap code
breaks bulk writes into multiple single-byte writes.
Signed-off-by: Thierry Reding <[email protected]>
---
drivers/rtc/rtc-max77686.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
index 35fd74b83626..f53db4d6bead 100644
--- a/drivers/rtc/rtc-max77686.c
+++ b/drivers/rtc/rtc-max77686.c
@@ -78,6 +78,8 @@ struct max77686_rtc_driver_data {
int alarm_pending_status_reg;
/* RTC IRQ CHIP for regmap */
const struct regmap_irq_chip *rtc_irq_chip;
+ /* regmap configuration for the chip */
+ const struct regmap_config *regmap_config;
};
struct max77686_rtc_info {
@@ -182,6 +184,11 @@ static const struct regmap_irq_chip max77686_rtc_irq_chip = {
.num_irqs = ARRAY_SIZE(max77686_rtc_irqs),
};
+static const struct regmap_config max77686_rtc_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+};
+
static const struct max77686_rtc_driver_data max77686_drv_data = {
.delay = 16000,
.mask = 0x7f,
@@ -191,6 +198,13 @@ static const struct max77686_rtc_driver_data max77686_drv_data = {
.alarm_pending_status_reg = MAX77686_REG_STATUS2,
.rtc_i2c_addr = MAX77686_I2C_ADDR_RTC,
.rtc_irq_chip = &max77686_rtc_irq_chip,
+ .regmap_config = &max77686_rtc_regmap_config,
+};
+
+static const struct regmap_config max77620_rtc_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .use_single_write = true,
};
static const struct max77686_rtc_driver_data max77620_drv_data = {
@@ -202,6 +216,7 @@ static const struct max77686_rtc_driver_data max77620_drv_data = {
.alarm_pending_status_reg = MAX77686_INVALID_REG,
.rtc_i2c_addr = MAX77620_I2C_ADDR_RTC,
.rtc_irq_chip = &max77686_rtc_irq_chip,
+ .regmap_config = &max77620_rtc_regmap_config,
};
static const unsigned int max77802_map[REG_RTC_END] = {
@@ -658,11 +673,6 @@ static int max77686_rtc_init_reg(struct max77686_rtc_info *info)
return ret;
}
-static const struct regmap_config max77686_rtc_regmap_config = {
- .reg_bits = 8,
- .val_bits = 8,
-};
-
static int max77686_init_rtc_regmap(struct max77686_rtc_info *info)
{
struct device *parent = info->dev->parent;
@@ -698,7 +708,7 @@ static int max77686_init_rtc_regmap(struct max77686_rtc_info *info)
}
info->rtc_regmap = devm_regmap_init_i2c(info->rtc,
- &max77686_rtc_regmap_config);
+ info->drv_data->regmap_config);
if (IS_ERR(info->rtc_regmap)) {
ret = PTR_ERR(info->rtc_regmap);
dev_err(info->dev, "Failed to allocate RTC regmap: %d\n", ret);
--
2.24.1
On 17/04/2020 18:08, Thierry Reding wrote:
> From: Thierry Reding <[email protected]>
>
> The MAX77620 doesn't support bulk writes, so make sure the regmap code
> breaks bulk writes into multiple single-byte writes.
>
> Signed-off-by: Thierry Reding <[email protected]>
> ---
> drivers/rtc/rtc-max77686.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/rtc/rtc-max77686.c b/drivers/rtc/rtc-max77686.c
> index 35fd74b83626..f53db4d6bead 100644
> --- a/drivers/rtc/rtc-max77686.c
> +++ b/drivers/rtc/rtc-max77686.c
> @@ -78,6 +78,8 @@ struct max77686_rtc_driver_data {
> int alarm_pending_status_reg;
> /* RTC IRQ CHIP for regmap */
> const struct regmap_irq_chip *rtc_irq_chip;
> + /* regmap configuration for the chip */
> + const struct regmap_config *regmap_config;
> };
>
> struct max77686_rtc_info {
> @@ -182,6 +184,11 @@ static const struct regmap_irq_chip max77686_rtc_irq_chip = {
> .num_irqs = ARRAY_SIZE(max77686_rtc_irqs),
> };
>
> +static const struct regmap_config max77686_rtc_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> +};
> +
> static const struct max77686_rtc_driver_data max77686_drv_data = {
> .delay = 16000,
> .mask = 0x7f,
> @@ -191,6 +198,13 @@ static const struct max77686_rtc_driver_data max77686_drv_data = {
> .alarm_pending_status_reg = MAX77686_REG_STATUS2,
> .rtc_i2c_addr = MAX77686_I2C_ADDR_RTC,
> .rtc_irq_chip = &max77686_rtc_irq_chip,
> + .regmap_config = &max77686_rtc_regmap_config,
> +};
> +
> +static const struct regmap_config max77620_rtc_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .use_single_write = true,
> };
>
> static const struct max77686_rtc_driver_data max77620_drv_data = {
> @@ -202,6 +216,7 @@ static const struct max77686_rtc_driver_data max77620_drv_data = {
> .alarm_pending_status_reg = MAX77686_INVALID_REG,
> .rtc_i2c_addr = MAX77620_I2C_ADDR_RTC,
> .rtc_irq_chip = &max77686_rtc_irq_chip,
> + .regmap_config = &max77620_rtc_regmap_config,
> };
>
> static const unsigned int max77802_map[REG_RTC_END] = {
> @@ -658,11 +673,6 @@ static int max77686_rtc_init_reg(struct max77686_rtc_info *info)
> return ret;
> }
>
> -static const struct regmap_config max77686_rtc_regmap_config = {
> - .reg_bits = 8,
> - .val_bits = 8,
> -};
> -
> static int max77686_init_rtc_regmap(struct max77686_rtc_info *info)
> {
> struct device *parent = info->dev->parent;
> @@ -698,7 +708,7 @@ static int max77686_init_rtc_regmap(struct max77686_rtc_info *info)
> }
>
> info->rtc_regmap = devm_regmap_init_i2c(info->rtc,
> - &max77686_rtc_regmap_config);
> + info->drv_data->regmap_config);
> if (IS_ERR(info->rtc_regmap)) {
> ret = PTR_ERR(info->rtc_regmap);
> dev_err(info->dev, "Failed to allocate RTC regmap: %d\n", ret);
>
Acked-by: Jon Hunter <[email protected]>
Tested-by: Jon Hunter <[email protected]>
Cheers
Jon
--
nvpublic
On Fri, Apr 17, 2020 at 07:08:23PM +0200, Thierry Reding wrote:
> From: Thierry Reding <[email protected]>
>
> 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 <[email protected]>
> ---
> .../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 <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > .../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.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
On Thu, Apr 30, 2020 at 9:15 AM Alexandre Belloni
<[email protected]> 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 <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > > .../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.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
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
On 17/04/2020 19:08:25+0200, Thierry Reding wrote:
> From: Thierry Reding <[email protected]>
>
> The MAX77620 doesn't support bulk writes, so make sure the regmap code
> breaks bulk writes into multiple single-byte writes.
>
> Signed-off-by: Thierry Reding <[email protected]>
> ---
> drivers/rtc/rtc-max77686.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
Applied, thanks.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
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.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com