diff mbox

[2/4] rtc: OMAP: Add system pm_power_off to rtc driver

Message ID 1352108549-9341-3-git-send-email-anilkumar@ti.com
State Superseded
Headers show

Commit Message

AnilKumar Chimata Nov. 5, 2012, 9:42 a.m. UTC
From: Colin Foe-Parker <colin.foeparker@logicpd.com>

Add system power off control to rtc driver which is the in-charge
of controlling the BeagleBone system power. The power_off routine
can be hooked up to "pm_power_off" system call.

System power off sequence:-
* Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
* Enable PMIC_POWER_EN in rtc module
* Set rtc ALARM2 time
* Enable ALARM2 interrupt

Added while (1); after the above steps to make sure that no other
process acquire cpu. Otherwise we might see an unexpected behaviour
because we are shutting down all the power rails of SoC except RTC.

Signed-off-by: Colin Foe-Parker <colin.foeparker@logicpd.com>
[anilkumar@ti.com: move poweroff additions to rtc driver]
Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 Documentation/devicetree/bindings/rtc/rtc-omap.txt |    5 ++
 drivers/rtc/rtc-omap.c                             |   79 +++++++++++++++++++-
 2 files changed, 83 insertions(+), 1 deletion(-)

Comments

Cousson, Benoit Nov. 5, 2012, 4:43 p.m. UTC | #1
Hi Anil / Colin,

On 11/05/2012 10:42 AM, AnilKumar Ch wrote:
> From: Colin Foe-Parker <colin.foeparker@logicpd.com>
> 
> Add system power off control to rtc driver which is the in-charge
> of controlling the BeagleBone system power. The power_off routine
> can be hooked up to "pm_power_off" system call.
> 
> System power off sequence:-
> * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
> * Enable PMIC_POWER_EN in rtc module
> * Set rtc ALARM2 time
> * Enable ALARM2 interrupt
> 
> Added while (1); after the above steps to make sure that no other
> process acquire cpu. Otherwise we might see an unexpected behaviour
> because we are shutting down all the power rails of SoC except RTC.
> 
> Signed-off-by: Colin Foe-Parker <colin.foeparker@logicpd.com>
> [anilkumar@ti.com: move poweroff additions to rtc driver]
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> ---
>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |    5 ++
>  drivers/rtc/rtc-omap.c                             |   79 +++++++++++++++++++-
>  2 files changed, 83 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> index b47aa41..8d9f4f9 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> @@ -6,6 +6,10 @@ Required properties:
>  - interrupts: rtc timer, alarm interrupts in order
>  - interrupt-parent: phandle for the interrupt controller
>  
> +Optional properties:
> +- ti,system-power-controller: Telling whether or not rtc is controlling
> +  the system power.

I don't know how it is connected at board level, but I'm not sure the
binding is the proper one.
It does not look super generic, and I'm wondering if we should not use
instead some regulator binding to reflect the connection of the RTC to a
regulator.

But without the board / soc spec it is hard to tell :-(

Regards,
Benoit


> +
>  Example:
>  
>  rtc@1c23000 {
> @@ -14,4 +18,5 @@ rtc@1c23000 {
>  	interrupts = <19
>  		      19>;
>  	interrupt-parent = <&intc>;
> +	ti,system-power-controller;
>  };
> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index 6009714..2d90170 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -72,6 +72,14 @@
>  #define OMAP_RTC_KICK0_REG		0x6c
>  #define OMAP_RTC_KICK1_REG		0x70
>  
> +#define OMAP_RTC_ALARM2_SECONDS_REG	0x80
> +#define OMAP_RTC_ALARM2_MINUTES_REG	0x84
> +#define OMAP_RTC_ALARM2_HOURS_REG	0x88
> +#define OMAP_RTC_ALARM2_DAYS_REG	0x8c
> +#define OMAP_RTC_ALARM2_MONTHS_REG	0x90
> +#define OMAP_RTC_ALARM2_YEARS_REG	0x94
> +#define OMAP_RTC_PMIC_REG		0x98
> +
>  /* OMAP_RTC_CTRL_REG bit fields: */
>  #define OMAP_RTC_CTRL_SPLIT		(1<<7)
>  #define OMAP_RTC_CTRL_DISABLE		(1<<6)
> @@ -93,15 +101,24 @@
>  #define OMAP_RTC_STATUS_BUSY            (1<<0)
>  
>  /* OMAP_RTC_INTERRUPTS_REG bit fields: */
> +#define OMAP_RTC_INTERRUPTS_IT_ALARM2   (1<<4)
>  #define OMAP_RTC_INTERRUPTS_IT_ALARM    (1<<3)
>  #define OMAP_RTC_INTERRUPTS_IT_TIMER    (1<<2)
>  
> +/* OMAP_RTC_PMIC_REG bit fields: */
> +#define OMAP_RTC_PMIC_POWER_EN_EN       (1<<16)
> +
>  /* OMAP_RTC_KICKER values */
>  #define	KICK0_VALUE			0x83e70b13
>  #define	KICK1_VALUE			0x95a4f1e0
>  
>  #define	OMAP_RTC_HAS_KICKER		0x1
>  
> +#define SHUTDOWN_TIME_SEC		2
> +#define SECS_IN_MIN			60
> +#define WAIT_AFTER			(SECS_IN_MIN - SHUTDOWN_TIME_SEC)
> +#define WAIT_TIME_MS			(SHUTDOWN_TIME_SEC * 1000)
> +
>  static void __iomem	*rtc_base;
>  
>  #define rtc_read(addr)		readb(rtc_base + (addr))
> @@ -290,6 +307,58 @@ static int omap_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
>  	return 0;
>  }
>  
> +/*
> + * rtc_power_off: Set the pmic power off sequence. The RTC generates
> + * pmic_pwr_enable control, which can be used to control an external
> + * PMIC.
> + */
> +static void rtc_power_off(void)
> +{
> +	u32 val;
> +	struct rtc_time tm;
> +	spinlock_t lock;
> +	unsigned long flags;
> +
> +	spin_lock_init(&lock);
> +
> +	/* Set PMIC power enable */
> +	val = readl(rtc_base + OMAP_RTC_PMIC_REG);
> +	writel(val | OMAP_RTC_PMIC_POWER_EN_EN, rtc_base + OMAP_RTC_PMIC_REG);
> +
> +	/* Wait few seconds instead of rollover */
> +	do {
> +		omap_rtc_read_time(NULL, &tm);
> +		if (WAIT_AFTER <= tm.tm_sec)
> +			mdelay(WAIT_TIME_MS);
> +	} while (WAIT_AFTER <= tm.tm_sec);
> +
> +	/* Add shutdown time to the current value */
> +	tm.tm_sec += SHUTDOWN_TIME_SEC;
> +
> +	if (tm2bcd(&tm) < 0)
> +		return;
> +
> +	pr_info("System will go to power_off state in approx. %d secs\n",
> +			SHUTDOWN_TIME_SEC);
> +
> +	/* Set the ALARM2 time */
> +	rtc_write(tm.tm_sec, OMAP_RTC_ALARM2_SECONDS_REG);
> +	rtc_write(tm.tm_min, OMAP_RTC_ALARM2_MINUTES_REG);
> +	rtc_write(tm.tm_hour, OMAP_RTC_ALARM2_HOURS_REG);
> +	rtc_write(tm.tm_mday, OMAP_RTC_ALARM2_DAYS_REG);
> +	rtc_write(tm.tm_mon, OMAP_RTC_ALARM2_MONTHS_REG);
> +	rtc_write(tm.tm_year, OMAP_RTC_ALARM2_YEARS_REG);
> +
> +	/* Enable alarm2 interrupt */
> +	val = readl(rtc_base + OMAP_RTC_INTERRUPTS_REG);
> +	writel(val | OMAP_RTC_INTERRUPTS_IT_ALARM2,
> +				rtc_base + OMAP_RTC_INTERRUPTS_REG);
> +
> +	/* Do not allow to execute any other task */
> +	spin_lock_irqsave(&lock, flags);
> +	while (1);
> +}
> +
>  static struct rtc_class_ops omap_rtc_ops = {
>  	.read_time	= omap_rtc_read_time,
>  	.set_time	= omap_rtc_set_time,
> @@ -327,12 +396,16 @@ static int __init omap_rtc_probe(struct platform_device *pdev)
>  	struct resource		*res, *mem;
>  	struct rtc_device	*rtc;
>  	u8			reg, new_ctrl;
> +	bool			pm_off = false;
>  	const struct platform_device_id *id_entry;
>  	const struct of_device_id *of_id;
>  
>  	of_id = of_match_device(omap_rtc_of_match, &pdev->dev);
> -	if (of_id)
> +	if (of_id) {
>  		pdev->id_entry = of_id->data;
> +		pm_off = of_property_read_bool(pdev->dev.of_node,
> +					"ti,system-power-controller");
> +	}
>  
>  	omap_rtc_timer = platform_get_irq(pdev, 0);
>  	if (omap_rtc_timer <= 0) {
> @@ -385,6 +458,10 @@ static int __init omap_rtc_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, rtc);
>  	dev_set_drvdata(&rtc->dev, mem);
>  
> +	/* RTC power off */
> +	if (pm_off && !pm_power_off)
> +		pm_power_off = rtc_power_off;
> +
>  	/* clear pending irqs, and set 1/second periodic,
>  	 * which we'll use instead of update irqs
>  	 */
>
Colin FoeParker Nov. 5, 2012, 5:39 p.m. UTC | #2
Hi Benoit,



"I don't know how it is connected at board level, but I'm not sure the binding is the proper one."



Here is a link to the Beaglebone schematic<http://beagleboard.org/static/beaglebone/latest/Docs/Hardware/BONE_SCH.pdf>.  The PMIC_PWR_EN net is the one that you should be interested in following.



In the AM335x, this net is connected to the ALARM2 event in the RTC subsystem.  If the ALARM2 event is triggered, and the PMIC_PWR_EN pin is configured correctly, the pin is driven low.



In the TPS65217 series PMIC, the PWR_EN I/O can be used to put the entire chip into a low power state.  This disables all the power rails in the PMIC with an exception of the rail that powers the RTC power domain.



So when the ALARM2 event occurs, the PWR_EN I/O is pulled low and the entire system into a low power "off" state.



"It does not look super generic, and I'm wondering if we should not use instead some regulator binding to reflect the connection of the RTC to a regulator.



But without the board / soc spec it is hard to tell :-("



This is a good point.  I know that there are other omap SOC RTC's that do not support the ALARM2 implementation.  That being said, I would argue that there is more of a RTC connection with the implementation than a regulator.



Anil will probably have greater insight into the pro's/con's of the implementation.



-Colin





Colin Foe-Parker // Software Engineer II







Logic PD /// engineering design services

5 Clock Tower Place. Suite 400

Maynard, MA 01754

T // 978.243.2045



colin.foeparker@logicpd.com

www.logicpd.com

/ / / / / / / / / / / / / / / / / / / / / / / / /

This message (including any attachments) contains confidential information intended for a specific individual and purpose, and is protected by law. If you are not the intended recipient, you should delete this message and are hereby notified that any disclosure, copying, distribution, or other use of this message, or the taking of any action based on it, is strictly prohibited.





-----Original Message-----
From: Benoit Cousson [mailto:b-cousson@ti.com]
Sent: Monday, November 05, 2012 11:43 AM
To: AnilKumar Ch; Colin FoeParker
Cc: a.zummo@towertech.it; sameo@linux.intel.com; tony@atomide.com; grant.likely@secretlab.ca; rob.herring@calxeda.com; rtc-linux@googlegroups.com; linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org; devicetree-discuss@lists.ozlabs.org
Subject: Re: [PATCH 2/4] rtc: OMAP: Add system pm_power_off to rtc driver



Hi Anil / Colin,



On 11/05/2012 10:42 AM, AnilKumar Ch wrote:

> From: Colin Foe-Parker <colin.foeparker@logicpd.com>

>

> Add system power off control to rtc driver which is the in-charge of

> controlling the BeagleBone system power. The power_off routine can be

> hooked up to "pm_power_off" system call.

>

> System power off sequence:-

> * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low

> * Enable PMIC_POWER_EN in rtc module

> * Set rtc ALARM2 time

> * Enable ALARM2 interrupt

>

> Added while (1); after the above steps to make sure that no other

> process acquire cpu. Otherwise we might see an unexpected behaviour

> because we are shutting down all the power rails of SoC except RTC.

>

> Signed-off-by: Colin Foe-Parker <colin.foeparker@logicpd.com<mailto:colin.foeparker@logicpd.com>>

> [anilkumar@ti.com: move poweroff additions to rtc driver]

> Signed-off-by: AnilKumar Ch <anilkumar@ti.com<mailto:anilkumar@ti.com>>

> ---

>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |    5 ++

>  drivers/rtc/rtc-omap.c                             |   79 +++++++++++++++++++-

>  2 files changed, 83 insertions(+), 1 deletion(-)

>

> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt

> b/Documentation/devicetree/bindings/rtc/rtc-omap.txt

> index b47aa41..8d9f4f9 100644

> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt

> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt

> @@ -6,6 +6,10 @@ Required properties:

>  - interrupts: rtc timer, alarm interrupts in order

>  - interrupt-parent: phandle for the interrupt controller

>

> +Optional properties:

> +- ti,system-power-controller: Telling whether or not rtc is

> +controlling

> +  the system power.



I don't know how it is connected at board level, but I'm not sure the binding is the proper one.

It does not look super generic, and I'm wondering if we should not use instead some regulator binding to reflect the connection of the RTC to a regulator.



But without the board / soc spec it is hard to tell :-(



Regards,

Benoit





> +

>  Example:

>

>  rtc@1c23000 {

> @@ -14,4 +18,5 @@ rtc@1c23000 {

>             interrupts = <19

>                                   19>;

>             interrupt-parent = <&intc>;

> +          ti,system-power-controller;

>  };

> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c index

> 6009714..2d90170 100644

> --- a/drivers/rtc/rtc-omap.c

> +++ b/drivers/rtc/rtc-omap.c

> @@ -72,6 +72,14 @@

>  #define OMAP_RTC_KICK0_REG                         0x6c

>  #define OMAP_RTC_KICK1_REG                         0x70

>

> +#define OMAP_RTC_ALARM2_SECONDS_REG            0x80

> +#define OMAP_RTC_ALARM2_MINUTES_REG            0x84

> +#define OMAP_RTC_ALARM2_HOURS_REG 0x88

> +#define OMAP_RTC_ALARM2_DAYS_REG     0x8c

> +#define OMAP_RTC_ALARM2_MONTHS_REG             0x90

> +#define OMAP_RTC_ALARM2_YEARS_REG   0x94

> +#define OMAP_RTC_PMIC_REG                        0x98

> +

>  /* OMAP_RTC_CTRL_REG bit fields: */

>  #define OMAP_RTC_CTRL_SPLIT                         (1<<7)

>  #define OMAP_RTC_CTRL_DISABLE                   (1<<6)

> @@ -93,15 +101,24 @@

>  #define OMAP_RTC_STATUS_BUSY            (1<<0)

>

>  /* OMAP_RTC_INTERRUPTS_REG bit fields: */

> +#define OMAP_RTC_INTERRUPTS_IT_ALARM2   (1<<4)

>  #define OMAP_RTC_INTERRUPTS_IT_ALARM    (1<<3)

>  #define OMAP_RTC_INTERRUPTS_IT_TIMER    (1<<2)

>

> +/* OMAP_RTC_PMIC_REG bit fields: */

> +#define OMAP_RTC_PMIC_POWER_EN_EN       (1<<16)

> +

>  /* OMAP_RTC_KICKER values */

>  #define           KICK0_VALUE                                    0x83e70b13

>  #define           KICK1_VALUE                                    0x95a4f1e0

>

>  #define           OMAP_RTC_HAS_KICKER                            0x1

>

> +#define SHUTDOWN_TIME_SEC                         2

> +#define SECS_IN_MIN                                            60

> +#define WAIT_AFTER                                              (SECS_IN_MIN - SHUTDOWN_TIME_SEC)

> +#define WAIT_TIME_MS                                       (SHUTDOWN_TIME_SEC * 1000)

> +

>  static void __iomem  *rtc_base;

>

>  #define rtc_read(addr)                            readb(rtc_base + (addr))

> @@ -290,6 +307,58 @@ static int omap_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)

>             return 0;

>  }

>

> +/*

> + * rtc_power_off: Set the pmic power off sequence. The RTC generates

> + * pmic_pwr_enable control, which can be used to control an external

> + * PMIC.

> + */

> +static void rtc_power_off(void)

> +{

> +          u32 val;

> +          struct rtc_time tm;

> +          spinlock_t lock;

> +          unsigned long flags;

> +

> +          spin_lock_init(&lock);

> +

> +          /* Set PMIC power enable */

> +          val = readl(rtc_base + OMAP_RTC_PMIC_REG);

> +          writel(val | OMAP_RTC_PMIC_POWER_EN_EN, rtc_base +

> +OMAP_RTC_PMIC_REG);

> +

> +          /* Wait few seconds instead of rollover */

> +          do {

> +                          omap_rtc_read_time(NULL, &tm);

> +                          if (WAIT_AFTER <= tm.tm_sec)

> +                                          mdelay(WAIT_TIME_MS);

> +          } while (WAIT_AFTER <= tm.tm_sec);

> +

> +          /* Add shutdown time to the current value */

> +          tm.tm_sec += SHUTDOWN_TIME_SEC;

> +

> +          if (tm2bcd(&tm) < 0)

> +                          return;

> +

> +          pr_info("System will go to power_off state in approx. %d secs\n",

> +                                          SHUTDOWN_TIME_SEC);

> +

> +          /* Set the ALARM2 time */

> +          rtc_write(tm.tm_sec, OMAP_RTC_ALARM2_SECONDS_REG);

> +          rtc_write(tm.tm_min, OMAP_RTC_ALARM2_MINUTES_REG);

> +          rtc_write(tm.tm_hour, OMAP_RTC_ALARM2_HOURS_REG);

> +          rtc_write(tm.tm_mday, OMAP_RTC_ALARM2_DAYS_REG);

> +          rtc_write(tm.tm_mon, OMAP_RTC_ALARM2_MONTHS_REG);

> +          rtc_write(tm.tm_year, OMAP_RTC_ALARM2_YEARS_REG);

> +

> +          /* Enable alarm2 interrupt */

> +          val = readl(rtc_base + OMAP_RTC_INTERRUPTS_REG);

> +          writel(val | OMAP_RTC_INTERRUPTS_IT_ALARM2,

> +                                                          rtc_base + OMAP_RTC_INTERRUPTS_REG);

> +

> +          /* Do not allow to execute any other task */

> +          spin_lock_irqsave(&lock, flags);

> +          while (1);

> +}

> +

>  static struct rtc_class_ops omap_rtc_ops = {

>             .read_time         = omap_rtc_read_time,

>             .set_time            = omap_rtc_set_time,

> @@ -327,12 +396,16 @@ static int __init omap_rtc_probe(struct platform_device *pdev)

>             struct resource                 *res, *mem;

>             struct rtc_device              *rtc;

>             u8                                           reg, new_ctrl;

> +          bool                                       pm_off = false;

>             const struct platform_device_id *id_entry;

>             const struct of_device_id *of_id;

>

>             of_id = of_match_device(omap_rtc_of_match, &pdev->dev);

> -           if (of_id)

> +          if (of_id) {

>                             pdev->id_entry = of_id->data;

> +                          pm_off = of_property_read_bool(pdev->dev.of_node,

> +                                                                          "ti,system-power-controller");

> +          }

>

>             omap_rtc_timer = platform_get_irq(pdev, 0);

>             if (omap_rtc_timer <= 0) {

> @@ -385,6 +458,10 @@ static int __init omap_rtc_probe(struct platform_device *pdev)

>             platform_set_drvdata(pdev, rtc);

>             dev_set_drvdata(&rtc->dev, mem);

>

> +          /* RTC power off */

> +          if (pm_off && !pm_power_off)

> +                          pm_power_off = rtc_power_off;

> +

>             /* clear pending irqs, and set 1/second periodic,

>              * which we'll use instead of update irqs

>              */

>
AnilKumar Chimata Nov. 6, 2012, 5:07 a.m. UTC | #3
On Mon, Nov 05, 2012 at 22:13:25, Cousson, Benoit wrote:
> Hi Anil / Colin,
> 
> On 11/05/2012 10:42 AM, AnilKumar Ch wrote:
> > From: Colin Foe-Parker <colin.foeparker@logicpd.com>
> > 
> > Add system power off control to rtc driver which is the in-charge
> > of controlling the BeagleBone system power. The power_off routine
> > can be hooked up to "pm_power_off" system call.
> > 
> > System power off sequence:-
> > * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
> > * Enable PMIC_POWER_EN in rtc module
> > * Set rtc ALARM2 time
> > * Enable ALARM2 interrupt
> > 
> > Added while (1); after the above steps to make sure that no other
> > process acquire cpu. Otherwise we might see an unexpected behaviour
> > because we are shutting down all the power rails of SoC except RTC.
> > 
> > Signed-off-by: Colin Foe-Parker <colin.foeparker@logicpd.com>
> > [anilkumar@ti.com: move poweroff additions to rtc driver]
> > Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> > ---
> >  Documentation/devicetree/bindings/rtc/rtc-omap.txt |    5 ++
> >  drivers/rtc/rtc-omap.c                             |   79 +++++++++++++++++++-
> >  2 files changed, 83 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > index b47aa41..8d9f4f9 100644
> > --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > @@ -6,6 +6,10 @@ Required properties:
> >  - interrupts: rtc timer, alarm interrupts in order
> >  - interrupt-parent: phandle for the interrupt controller
> >  
> > +Optional properties:
> > +- ti,system-power-controller: Telling whether or not rtc is controlling
> > +  the system power.
> 
> I don't know how it is connected at board level, but I'm not sure the
> binding is the proper one.

Hi Benoit,
 ________________________________
|   ______          _______      |
|  |      |        |       |     |
|  |RTC   |        |       |     |
|  |PMIC  |  Line  |       |     |
|  |PWR_EN|=======>|PWR_EN |     |
|  |______|        |_______|     |
|  AM335x SoC       TPS65217     |
|                                |
|________________________________|
          BeagleBone

This is how RTC PMIC_PWR_EN is connected to PWR_EN of TPS65217 PMIC. Only when
RTC pull low in PMIC_PWR_EN then PMIC will go to power off state provided TPS65217
status should be changed to STATUS_OFF.

ALARM2 event should be trigger to configure PMIC_PWR_EN properly then the "Line"
driven low so that PMIC will go to shutdown mode.

Thanks
AnilKumar

> It does not look super generic, and I'm wondering if we should not use
> instead some regulator binding to reflect the connection of the RTC to a
> regulator.
> 
> But without the board / soc spec it is hard to tell :-(
>
> Regards,
> Benoit
> 
> 
> > +
> >  Example:
> >  
> >  rtc@1c23000 {
> > @@ -14,4 +18,5 @@ rtc@1c23000 {
> >  	interrupts = <19
> >  		      19>;
> >  	interrupt-parent = <&intc>;
> > +	ti,system-power-controller;
> >  };
> > diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> > index 6009714..2d90170 100644
> > --- a/drivers/rtc/rtc-omap.c
> > +++ b/drivers/rtc/rtc-omap.c
> > @@ -72,6 +72,14 @@
> >  #define OMAP_RTC_KICK0_REG		0x6c
> >  #define OMAP_RTC_KICK1_REG		0x70
> >  
> > +#define OMAP_RTC_ALARM2_SECONDS_REG	0x80
> > +#define OMAP_RTC_ALARM2_MINUTES_REG	0x84
> > +#define OMAP_RTC_ALARM2_HOURS_REG	0x88
> > +#define OMAP_RTC_ALARM2_DAYS_REG	0x8c
> > +#define OMAP_RTC_ALARM2_MONTHS_REG	0x90
> > +#define OMAP_RTC_ALARM2_YEARS_REG	0x94
> > +#define OMAP_RTC_PMIC_REG		0x98
> > +
> >  /* OMAP_RTC_CTRL_REG bit fields: */
> >  #define OMAP_RTC_CTRL_SPLIT		(1<<7)
> >  #define OMAP_RTC_CTRL_DISABLE		(1<<6)
> > @@ -93,15 +101,24 @@
> >  #define OMAP_RTC_STATUS_BUSY            (1<<0)
> >  
> >  /* OMAP_RTC_INTERRUPTS_REG bit fields: */
> > +#define OMAP_RTC_INTERRUPTS_IT_ALARM2   (1<<4)
> >  #define OMAP_RTC_INTERRUPTS_IT_ALARM    (1<<3)
> >  #define OMAP_RTC_INTERRUPTS_IT_TIMER    (1<<2)
> >  
> > +/* OMAP_RTC_PMIC_REG bit fields: */
> > +#define OMAP_RTC_PMIC_POWER_EN_EN       (1<<16)
> > +
> >  /* OMAP_RTC_KICKER values */
> >  #define	KICK0_VALUE			0x83e70b13
> >  #define	KICK1_VALUE			0x95a4f1e0
> >  
> >  #define	OMAP_RTC_HAS_KICKER		0x1
> >  
> > +#define SHUTDOWN_TIME_SEC		2
> > +#define SECS_IN_MIN			60
> > +#define WAIT_AFTER			(SECS_IN_MIN - SHUTDOWN_TIME_SEC)
> > +#define WAIT_TIME_MS			(SHUTDOWN_TIME_SEC * 1000)
> > +
> >  static void __iomem	*rtc_base;
> >  
> >  #define rtc_read(addr)		readb(rtc_base + (addr))
> > @@ -290,6 +307,58 @@ static int omap_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
> >  	return 0;
> >  }
> >  
> > +/*
> > + * rtc_power_off: Set the pmic power off sequence. The RTC generates
> > + * pmic_pwr_enable control, which can be used to control an external
> > + * PMIC.
> > + */
> > +static void rtc_power_off(void)
> > +{
> > +	u32 val;
> > +	struct rtc_time tm;
> > +	spinlock_t lock;
> > +	unsigned long flags;
> > +
> > +	spin_lock_init(&lock);
> > +
> > +	/* Set PMIC power enable */
> > +	val = readl(rtc_base + OMAP_RTC_PMIC_REG);
> > +	writel(val | OMAP_RTC_PMIC_POWER_EN_EN, rtc_base + OMAP_RTC_PMIC_REG);
> > +
> > +	/* Wait few seconds instead of rollover */
> > +	do {
> > +		omap_rtc_read_time(NULL, &tm);
> > +		if (WAIT_AFTER <= tm.tm_sec)
> > +			mdelay(WAIT_TIME_MS);
> > +	} while (WAIT_AFTER <= tm.tm_sec);
> > +
> > +	/* Add shutdown time to the current value */
> > +	tm.tm_sec += SHUTDOWN_TIME_SEC;
> > +
> > +	if (tm2bcd(&tm) < 0)
> > +		return;
> > +
> > +	pr_info("System will go to power_off state in approx. %d secs\n",
> > +			SHUTDOWN_TIME_SEC);
> > +
> > +	/* Set the ALARM2 time */
> > +	rtc_write(tm.tm_sec, OMAP_RTC_ALARM2_SECONDS_REG);
> > +	rtc_write(tm.tm_min, OMAP_RTC_ALARM2_MINUTES_REG);
> > +	rtc_write(tm.tm_hour, OMAP_RTC_ALARM2_HOURS_REG);
> > +	rtc_write(tm.tm_mday, OMAP_RTC_ALARM2_DAYS_REG);
> > +	rtc_write(tm.tm_mon, OMAP_RTC_ALARM2_MONTHS_REG);
> > +	rtc_write(tm.tm_year, OMAP_RTC_ALARM2_YEARS_REG);
> > +
> > +	/* Enable alarm2 interrupt */
> > +	val = readl(rtc_base + OMAP_RTC_INTERRUPTS_REG);
> > +	writel(val | OMAP_RTC_INTERRUPTS_IT_ALARM2,
> > +				rtc_base + OMAP_RTC_INTERRUPTS_REG);
> > +
> > +	/* Do not allow to execute any other task */
> > +	spin_lock_irqsave(&lock, flags);
> > +	while (1);
> > +}
> > +
> >  static struct rtc_class_ops omap_rtc_ops = {
> >  	.read_time	= omap_rtc_read_time,
> >  	.set_time	= omap_rtc_set_time,
> > @@ -327,12 +396,16 @@ static int __init omap_rtc_probe(struct platform_device *pdev)
> >  	struct resource		*res, *mem;
> >  	struct rtc_device	*rtc;
> >  	u8			reg, new_ctrl;
> > +	bool			pm_off = false;
> >  	const struct platform_device_id *id_entry;
> >  	const struct of_device_id *of_id;
> >  
> >  	of_id = of_match_device(omap_rtc_of_match, &pdev->dev);
> > -	if (of_id)
> > +	if (of_id) {
> >  		pdev->id_entry = of_id->data;
> > +		pm_off = of_property_read_bool(pdev->dev.of_node,
> > +					"ti,system-power-controller");
> > +	}
> >  
> >  	omap_rtc_timer = platform_get_irq(pdev, 0);
> >  	if (omap_rtc_timer <= 0) {
> > @@ -385,6 +458,10 @@ static int __init omap_rtc_probe(struct platform_device *pdev)
> >  	platform_set_drvdata(pdev, rtc);
> >  	dev_set_drvdata(&rtc->dev, mem);
> >  
> > +	/* RTC power off */
> > +	if (pm_off && !pm_power_off)
> > +		pm_power_off = rtc_power_off;
> > +
> >  	/* clear pending irqs, and set 1/second periodic,
> >  	 * which we'll use instead of update irqs
> >  	 */
> > 
> 
>
Bedia, Vaibhav Nov. 6, 2012, 5:45 a.m. UTC | #4
On Mon, Nov 05, 2012 at 15:12:27, AnilKumar, Chimata wrote:
[...]
>  
> +#define SHUTDOWN_TIME_SEC		2
> +#define SECS_IN_MIN			60
> +#define WAIT_AFTER			(SECS_IN_MIN - SHUTDOWN_TIME_SEC)
> +#define WAIT_TIME_MS			(SHUTDOWN_TIME_SEC * 1000)
> +
>  static void __iomem	*rtc_base;
>  
[...]
> +
> +	/* Wait few seconds instead of rollover */
> +	do {
> +		omap_rtc_read_time(NULL, &tm);
> +		if (WAIT_AFTER <= tm.tm_sec)
> +			mdelay(WAIT_TIME_MS);
> +	} while (WAIT_AFTER <= tm.tm_sec);

This hardcoded wait for rollover doesn't look good. I see some
helper functions in rtc-lib.c which probably could be used for
converting the current time to elapsed seconds, add the delay and
then convert it back to the time to be programmed in RTC without
worrying about rollover. Why not use that?

> +
> +	/* Add shutdown time to the current value */
> +	tm.tm_sec += SHUTDOWN_TIME_SEC;
> +
> +	if (tm2bcd(&tm) < 0)
> +		return;
> +
> +	pr_info("System will go to power_off state in approx. %d secs\n",
> +			SHUTDOWN_TIME_SEC);
> +
> +	/* Set the ALARM2 time */
> +	rtc_write(tm.tm_sec, OMAP_RTC_ALARM2_SECONDS_REG);
> +	rtc_write(tm.tm_min, OMAP_RTC_ALARM2_MINUTES_REG);
> +	rtc_write(tm.tm_hour, OMAP_RTC_ALARM2_HOURS_REG);
> +	rtc_write(tm.tm_mday, OMAP_RTC_ALARM2_DAYS_REG);
> +	rtc_write(tm.tm_mon, OMAP_RTC_ALARM2_MONTHS_REG);
> +	rtc_write(tm.tm_year, OMAP_RTC_ALARM2_YEARS_REG);
> +
> +	/* Enable alarm2 interrupt */
> +	val = readl(rtc_base + OMAP_RTC_INTERRUPTS_REG);
> +	writel(val | OMAP_RTC_INTERRUPTS_IT_ALARM2,
> +				rtc_base + OMAP_RTC_INTERRUPTS_REG);
> +

These registers are not present in older versions of the IP so how
does that get handled?

You also need to describe the connection between the ALARM2 and the
power off logic in detail.

Regards,
Vaibhav
Cousson, Benoit Nov. 6, 2012, 4:56 p.m. UTC | #5
Hi Anil,

On 11/06/2012 06:07 AM, AnilKumar, Chimata wrote:
> On Mon, Nov 05, 2012 at 22:13:25, Cousson, Benoit wrote:
>> Hi Anil / Colin,
>>
>> On 11/05/2012 10:42 AM, AnilKumar Ch wrote:
>>> From: Colin Foe-Parker <colin.foeparker@logicpd.com>
>>>
>>> Add system power off control to rtc driver which is the in-charge
>>> of controlling the BeagleBone system power. The power_off routine
>>> can be hooked up to "pm_power_off" system call.
>>>
>>> System power off sequence:-
>>> * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
>>> * Enable PMIC_POWER_EN in rtc module
>>> * Set rtc ALARM2 time
>>> * Enable ALARM2 interrupt
>>>
>>> Added while (1); after the above steps to make sure that no other
>>> process acquire cpu. Otherwise we might see an unexpected behaviour
>>> because we are shutting down all the power rails of SoC except RTC.
>>>
>>> Signed-off-by: Colin Foe-Parker <colin.foeparker@logicpd.com>
>>> [anilkumar@ti.com: move poweroff additions to rtc driver]
>>> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
>>> ---
>>>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |    5 ++
>>>  drivers/rtc/rtc-omap.c                             |   79 +++++++++++++++++++-
>>>  2 files changed, 83 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
>>> index b47aa41..8d9f4f9 100644
>>> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
>>> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
>>> @@ -6,6 +6,10 @@ Required properties:
>>>  - interrupts: rtc timer, alarm interrupts in order
>>>  - interrupt-parent: phandle for the interrupt controller
>>>  
>>> +Optional properties:
>>> +- ti,system-power-controller: Telling whether or not rtc is controlling
>>> +  the system power.
>>
>> I don't know how it is connected at board level, but I'm not sure the
>> binding is the proper one.
> 
> Hi Benoit,
>  ________________________________
> |   ______          _______      |
> |  |      |        |       |     |
> |  |RTC   |        |       |     |
> |  |PMIC  |  Line  |       |     |
> |  |PWR_EN|=======>|PWR_EN |     |
> |  |______|        |_______|     |
> |  AM335x SoC       TPS65217     |
> |                                |
> |________________________________|
>           BeagleBone
> 
> This is how RTC PMIC_PWR_EN is connected to PWR_EN of TPS65217 PMIC. Only when
> RTC pull low in PMIC_PWR_EN then PMIC will go to power off state provided TPS65217
> status should be changed to STATUS_OFF.
> 
> ALARM2 event should be trigger to configure PMIC_PWR_EN properly then the "Line"
> driven low so that PMIC will go to shutdown mode.

Thanks for the nice diagram :-)

I'm wondering if we cannot abuse the gpio binding to describe that
connection instead of creating two custom attributes (PMIC + RTC).

Ideally we should do that without having to change the RTC to use the
gpiolib at all.


rtc: rtc@44e3e000 {
	compatible = "ti,da830-rtc";
	reg = <0x44e3e000 0x1000>;
	interrupts = <75, 76>;
	ti,hwmods = "rtc";

	/* expose the PWR_EN functionality of this RTC*/
	gpio-controller;
	#gpio-cells = <0>; /* assuming we can use 0 ??? */
};

...

tps: tps@24 {
 	compatible = "ti,tps65217";
	/*
         * Enable the power enable feature from
         * the input line if that attribute is there.
         */
	gpio-power-en = <&rtc>; /* PWR_EN */

	...
}	

Any thought?

Regards,
Benoit
AnilKumar Chimata Nov. 12, 2012, 9:47 a.m. UTC | #6
On Tue, Nov 06, 2012 at 22:26:54, Cousson, Benoit wrote:
> Hi Anil,
> 
> On 11/06/2012 06:07 AM, AnilKumar, Chimata wrote:
> > On Mon, Nov 05, 2012 at 22:13:25, Cousson, Benoit wrote:
> >> Hi Anil / Colin,
> >>
> >> On 11/05/2012 10:42 AM, AnilKumar Ch wrote:
> >>> From: Colin Foe-Parker <colin.foeparker@logicpd.com>
> >>>
> >>> Add system power off control to rtc driver which is the in-charge
> >>> of controlling the BeagleBone system power. The power_off routine
> >>> can be hooked up to "pm_power_off" system call.
> >>>
> >>> System power off sequence:-
> >>> * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
> >>> * Enable PMIC_POWER_EN in rtc module
> >>> * Set rtc ALARM2 time
> >>> * Enable ALARM2 interrupt
> >>>
> >>> Added while (1); after the above steps to make sure that no other
> >>> process acquire cpu. Otherwise we might see an unexpected behaviour
> >>> because we are shutting down all the power rails of SoC except RTC.
> >>>
> >>> Signed-off-by: Colin Foe-Parker <colin.foeparker@logicpd.com>
> >>> [anilkumar@ti.com: move poweroff additions to rtc driver]
> >>> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> >>> ---
> >>>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |    5 ++
> >>>  drivers/rtc/rtc-omap.c                             |   79 +++++++++++++++++++-
> >>>  2 files changed, 83 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> >>> index b47aa41..8d9f4f9 100644
> >>> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> >>> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> >>> @@ -6,6 +6,10 @@ Required properties:
> >>>  - interrupts: rtc timer, alarm interrupts in order
> >>>  - interrupt-parent: phandle for the interrupt controller
> >>>  
> >>> +Optional properties:
> >>> +- ti,system-power-controller: Telling whether or not rtc is controlling
> >>> +  the system power.
> >>
> >> I don't know how it is connected at board level, but I'm not sure the
> >> binding is the proper one.
> > 
> > Hi Benoit,
> >  ________________________________
> > |   ______          _______      |
> > |  |      |        |       |     |
> > |  |RTC   |        |       |     |
> > |  |PMIC  |  Line  |       |     |
> > |  |PWR_EN|=======>|PWR_EN |     |
> > |  |______|        |_______|     |
> > |  AM335x SoC       TPS65217     |
> > |                                |
> > |________________________________|
> >           BeagleBone
> > 
> > This is how RTC PMIC_PWR_EN is connected to PWR_EN of TPS65217 PMIC. Only when
> > RTC pull low in PMIC_PWR_EN then PMIC will go to power off state provided TPS65217
> > status should be changed to STATUS_OFF.
> > 
> > ALARM2 event should be trigger to configure PMIC_PWR_EN properly then the "Line"
> > driven low so that PMIC will go to shutdown mode.
> 
> Thanks for the nice diagram :-)

I missed this mail thread so delayed in response

> 
> I'm wondering if we cannot abuse the gpio binding to describe that
> connection instead of creating two custom attributes (PMIC + RTC).
> 
> Ideally we should do that without having to change the RTC to use the
> gpiolib at all.
> 
> 
> rtc: rtc@44e3e000 {
> 	compatible = "ti,da830-rtc";
> 	reg = <0x44e3e000 0x1000>;
> 	interrupts = <75, 76>;
> 	ti,hwmods = "rtc";
> 
> 	/* expose the PWR_EN functionality of this RTC*/
> 	gpio-controller;
> 	#gpio-cells = <0>; /* assuming we can use 0 ??? */
> };
> 
> ...
> 
> tps: tps@24 {
>  	compatible = "ti,tps65217";
> 	/*
>          * Enable the power enable feature from
>          * the input line if that attribute is there.
>          */
> 	gpio-power-en = <&rtc>; /* PWR_EN */
> 
> 	...
> }	
> 
> Any thought?

No, these two are independent controllers. PMIC can go to power
off mode if we pull PWR_EN to low. We can pull down that line
by any means like PRCM or GPIO or some other. So these two flags
should be independent from each other.

Thanks
AnilKumar
AnilKumar Chimata Nov. 12, 2012, 9:47 a.m. UTC | #7
On Tue, Nov 06, 2012 at 11:15:34, Bedia, Vaibhav wrote:
> On Mon, Nov 05, 2012 at 15:12:27, AnilKumar, Chimata wrote:
> [...]
> >  
> > +#define SHUTDOWN_TIME_SEC		2
> > +#define SECS_IN_MIN			60
> > +#define WAIT_AFTER			(SECS_IN_MIN - SHUTDOWN_TIME_SEC)
> > +#define WAIT_TIME_MS			(SHUTDOWN_TIME_SEC * 1000)
> > +
> >  static void __iomem	*rtc_base;
> >  
> [...]
> > +
> > +	/* Wait few seconds instead of rollover */
> > +	do {
> > +		omap_rtc_read_time(NULL, &tm);
> > +		if (WAIT_AFTER <= tm.tm_sec)
> > +			mdelay(WAIT_TIME_MS);
> > +	} while (WAIT_AFTER <= tm.tm_sec);
> 
> This hardcoded wait for rollover doesn't look good. I see some
> helper functions in rtc-lib.c which probably could be used for
> converting the current time to elapsed seconds, add the delay and
> then convert it back to the time to be programmed in RTC without
> worrying about rollover. Why not use that?

I am not aware of those APIs, can you point some?

> 
> > +
> > +	/* Add shutdown time to the current value */
> > +	tm.tm_sec += SHUTDOWN_TIME_SEC;
> > +
> > +	if (tm2bcd(&tm) < 0)
> > +		return;
> > +
> > +	pr_info("System will go to power_off state in approx. %d secs\n",
> > +			SHUTDOWN_TIME_SEC);
> > +
> > +	/* Set the ALARM2 time */
> > +	rtc_write(tm.tm_sec, OMAP_RTC_ALARM2_SECONDS_REG);
> > +	rtc_write(tm.tm_min, OMAP_RTC_ALARM2_MINUTES_REG);
> > +	rtc_write(tm.tm_hour, OMAP_RTC_ALARM2_HOURS_REG);
> > +	rtc_write(tm.tm_mday, OMAP_RTC_ALARM2_DAYS_REG);
> > +	rtc_write(tm.tm_mon, OMAP_RTC_ALARM2_MONTHS_REG);
> > +	rtc_write(tm.tm_year, OMAP_RTC_ALARM2_YEARS_REG);
> > +
> > +	/* Enable alarm2 interrupt */
> > +	val = readl(rtc_base + OMAP_RTC_INTERRUPTS_REG);
> > +	writel(val | OMAP_RTC_INTERRUPTS_IT_ALARM2,
> > +				rtc_base + OMAP_RTC_INTERRUPTS_REG);
> > +
> 
> These registers are not present in older versions of the IP so how
> does that get handled?

I think, earlier this feature is not supported/not used.

> 
> You also need to describe the connection between the ALARM2 and the
> power off logic in detail.

Sure, I will add.

Thanks
AnilKumar
AnilKumar Chimata Nov. 14, 2012, 5:01 a.m. UTC | #8
+Mark

On Mon, Nov 12, 2012 at 15:17:13, AnilKumar, Chimata wrote:
> On Tue, Nov 06, 2012 at 22:26:54, Cousson, Benoit wrote:
> > Hi Anil,
> > 
> > On 11/06/2012 06:07 AM, AnilKumar, Chimata wrote:
> > > On Mon, Nov 05, 2012 at 22:13:25, Cousson, Benoit wrote:
> > >> Hi Anil / Colin,
> > >>
> > >> On 11/05/2012 10:42 AM, AnilKumar Ch wrote:
> > >>> From: Colin Foe-Parker <colin.foeparker@logicpd.com>
> > >>>
> > >>> Add system power off control to rtc driver which is the in-charge
> > >>> of controlling the BeagleBone system power. The power_off routine
> > >>> can be hooked up to "pm_power_off" system call.
> > >>>
> > >>> System power off sequence:-
> > >>> * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
> > >>> * Enable PMIC_POWER_EN in rtc module
> > >>> * Set rtc ALARM2 time
> > >>> * Enable ALARM2 interrupt
> > >>>
> > >>> Added while (1); after the above steps to make sure that no other
> > >>> process acquire cpu. Otherwise we might see an unexpected behaviour
> > >>> because we are shutting down all the power rails of SoC except RTC.
> > >>>
> > >>> Signed-off-by: Colin Foe-Parker <colin.foeparker@logicpd.com>
> > >>> [anilkumar@ti.com: move poweroff additions to rtc driver]
> > >>> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> > >>> ---
> > >>>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |    5 ++
> > >>>  drivers/rtc/rtc-omap.c                             |   79 +++++++++++++++++++-
> > >>>  2 files changed, 83 insertions(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > >>> index b47aa41..8d9f4f9 100644
> > >>> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > >>> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > >>> @@ -6,6 +6,10 @@ Required properties:
> > >>>  - interrupts: rtc timer, alarm interrupts in order
> > >>>  - interrupt-parent: phandle for the interrupt controller
> > >>>  
> > >>> +Optional properties:
> > >>> +- ti,system-power-controller: Telling whether or not rtc is controlling
> > >>> +  the system power.
> > >>
> > >> I don't know how it is connected at board level, but I'm not sure the
> > >> binding is the proper one.
> > > 
> > > Hi Benoit,
> > >  ________________________________
> > > |   ______          _______      |
> > > |  |      |        |       |     |
> > > |  |RTC   |        |       |     |
> > > |  |PMIC  |  Line  |       |     |
> > > |  |PWR_EN|=======>|PWR_EN |     |
> > > |  |______|        |_______|     |
> > > |  AM335x SoC       TPS65217     |
> > > |                                |
> > > |________________________________|
> > >           BeagleBone
> > > 
> > > This is how RTC PMIC_PWR_EN is connected to PWR_EN of TPS65217 PMIC. Only when
> > > RTC pull low in PMIC_PWR_EN then PMIC will go to power off state provided TPS65217
> > > status should be changed to STATUS_OFF.
> > > 
> > > ALARM2 event should be trigger to configure PMIC_PWR_EN properly then the "Line"
> > > driven low so that PMIC will go to shutdown mode.
> > 
> > Thanks for the nice diagram :-)
> 
> I missed this mail thread so delayed in response
> 
> > 
> > I'm wondering if we cannot abuse the gpio binding to describe that
> > connection instead of creating two custom attributes (PMIC + RTC).
> > 
> > Ideally we should do that without having to change the RTC to use the
> > gpiolib at all.
> > 
> > 
> > rtc: rtc@44e3e000 {
> > 	compatible = "ti,da830-rtc";
> > 	reg = <0x44e3e000 0x1000>;
> > 	interrupts = <75, 76>;
> > 	ti,hwmods = "rtc";
> > 
> > 	/* expose the PWR_EN functionality of this RTC*/
> > 	gpio-controller;
> > 	#gpio-cells = <0>; /* assuming we can use 0 ??? */
> > };
> > 
> > ...
> > 
> > tps: tps@24 {
> >  	compatible = "ti,tps65217";
> > 	/*
> >          * Enable the power enable feature from
> >          * the input line if that attribute is there.
> >          */
> > 	gpio-power-en = <&rtc>; /* PWR_EN */
> > 
> > 	...
> > }	
> > 
> > Any thought?
> 
> No, these two are independent controllers. PMIC can go to power
> off mode if we pull PWR_EN to low. We can pull down that line
> by any means like PRCM or GPIO or some other. So these two flags
> should be independent from each other.
> 
> Thanks
> AnilKumar
>
Mark Brown Nov. 14, 2012, 5:21 a.m. UTC | #9
On Wed, Nov 14, 2012 at 05:01:45AM +0000, AnilKumar, Chimata wrote:
> +Mark

...without any words as to why I'm being added or what the content in
the message is?
AnilKumar Chimata Nov. 14, 2012, 5:50 a.m. UTC | #10
On Wed, Nov 14, 2012 at 10:31:42, AnilKumar, Chimata wrote:
> +Mark
> 
> On Mon, Nov 12, 2012 at 15:17:13, AnilKumar, Chimata wrote:
> > On Tue, Nov 06, 2012 at 22:26:54, Cousson, Benoit wrote:
> > > Hi Anil,
> > > 
> > > On 11/06/2012 06:07 AM, AnilKumar, Chimata wrote:
> > > > On Mon, Nov 05, 2012 at 22:13:25, Cousson, Benoit wrote:
> > > >> Hi Anil / Colin,
> > > >>
> > > >> On 11/05/2012 10:42 AM, AnilKumar Ch wrote:
> > > >>> From: Colin Foe-Parker <colin.foeparker@logicpd.com>
> > > >>>
> > > >>> Add system power off control to rtc driver which is the in-charge
> > > >>> of controlling the BeagleBone system power. The power_off routine
> > > >>> can be hooked up to "pm_power_off" system call.
> > > >>>
> > > >>> System power off sequence:-
> > > >>> * Set PMIC STATUS_OFF when PMIC_POWER_EN is pulled low
> > > >>> * Enable PMIC_POWER_EN in rtc module
> > > >>> * Set rtc ALARM2 time
> > > >>> * Enable ALARM2 interrupt
> > > >>>
> > > >>> Added while (1); after the above steps to make sure that no other
> > > >>> process acquire cpu. Otherwise we might see an unexpected behaviour
> > > >>> because we are shutting down all the power rails of SoC except RTC.
> > > >>>
> > > >>> Signed-off-by: Colin Foe-Parker <colin.foeparker@logicpd.com>
> > > >>> [anilkumar@ti.com: move poweroff additions to rtc driver]
> > > >>> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> > > >>> ---
> > > >>>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |    5 ++
> > > >>>  drivers/rtc/rtc-omap.c                             |   79 +++++++++++++++++++-
> > > >>>  2 files changed, 83 insertions(+), 1 deletion(-)
> > > >>>
> > > >>> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > > >>> index b47aa41..8d9f4f9 100644
> > > >>> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > > >>> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > > >>> @@ -6,6 +6,10 @@ Required properties:
> > > >>>  - interrupts: rtc timer, alarm interrupts in order
> > > >>>  - interrupt-parent: phandle for the interrupt controller
> > > >>>  
> > > >>> +Optional properties:
> > > >>> +- ti,system-power-controller: Telling whether or not rtc is controlling
> > > >>> +  the system power.
> > > >>
> > > >> I don't know how it is connected at board level, but I'm not sure the
> > > >> binding is the proper one.
> > > > 
> > > > Hi Benoit,
> > > >  ________________________________
> > > > |   ______          _______      |
> > > > |  |      |        |       |     |
> > > > |  |RTC   |        |       |     |
> > > > |  |PMIC  |  Line  |       |     |
> > > > |  |PWR_EN|=======>|PWR_EN |     |
> > > > |  |______|        |_______|     |
> > > > |  AM335x SoC       TPS65217     |
> > > > |                                |
> > > > |________________________________|
> > > >           BeagleBone
> > > > 
> > > > This is how RTC PMIC_PWR_EN is connected to PWR_EN of TPS65217 PMIC. Only when
> > > > RTC pull low in PMIC_PWR_EN then PMIC will go to power off state provided TPS65217
> > > > status should be changed to STATUS_OFF.
> > > > 
> > > > ALARM2 event should be trigger to configure PMIC_PWR_EN properly then the "Line"
> > > > driven low so that PMIC will go to shutdown mode.

Mark,

Details regarding how PMIC PWR_EN is connected to RTC module

Thanks
AnilKumar

> > > 
> > > Thanks for the nice diagram :-)
> > 
> > I missed this mail thread so delayed in response
> > 
> > > 
> > > I'm wondering if we cannot abuse the gpio binding to describe that
> > > connection instead of creating two custom attributes (PMIC + RTC).
> > > 
> > > Ideally we should do that without having to change the RTC to use the
> > > gpiolib at all.
> > > 
> > > 
> > > rtc: rtc@44e3e000 {
> > > 	compatible = "ti,da830-rtc";
> > > 	reg = <0x44e3e000 0x1000>;
> > > 	interrupts = <75, 76>;
> > > 	ti,hwmods = "rtc";
> > > 
> > > 	/* expose the PWR_EN functionality of this RTC*/
> > > 	gpio-controller;
> > > 	#gpio-cells = <0>; /* assuming we can use 0 ??? */
> > > };
> > > 
> > > ...
> > > 
> > > tps: tps@24 {
> > >  	compatible = "ti,tps65217";
> > > 	/*
> > >          * Enable the power enable feature from
> > >          * the input line if that attribute is there.
> > >          */
> > > 	gpio-power-en = <&rtc>; /* PWR_EN */
> > > 
> > > 	...
> > > }	
> > > 
> > > Any thought?
> > 
> > No, these two are independent controllers. PMIC can go to power
> > off mode if we pull PWR_EN to low. We can pull down that line
> > by any means like PRCM or GPIO or some other. So these two flags
> > should be independent from each other.
> > 
> > Thanks
> > AnilKumar
> > 
> 
>
Mark Brown Nov. 14, 2012, 6 a.m. UTC | #11
On Wed, Nov 14, 2012 at 05:50:46AM +0000, AnilKumar, Chimata wrote:
> On Wed, Nov 14, 2012 at 10:31:42, AnilKumar, Chimata wrote:

*Please* cut out irrelevant context from your mails.

> Details regarding how PMIC PWR_EN is connected to RTC module

OK...  but what am I expected to do with this information?
AnilKumar Chimata Nov. 16, 2012, 6:13 a.m. UTC | #12
On Mon, Nov 12, 2012 at 15:17:48, AnilKumar, Chimata wrote:
> On Tue, Nov 06, 2012 at 11:15:34, Bedia, Vaibhav wrote:
> > On Mon, Nov 05, 2012 at 15:12:27, AnilKumar, Chimata wrote:
> > [...]
> > >  
> > > +#define SHUTDOWN_TIME_SEC		2
> > > +#define SECS_IN_MIN			60
> > > +#define WAIT_AFTER			(SECS_IN_MIN - SHUTDOWN_TIME_SEC)
> > > +#define WAIT_TIME_MS			(SHUTDOWN_TIME_SEC * 1000)
> > > +
> > >  static void __iomem	*rtc_base;
> > >  
> > [...]
> > > +
> > > +	/* Wait few seconds instead of rollover */
> > > +	do {
> > > +		omap_rtc_read_time(NULL, &tm);
> > > +		if (WAIT_AFTER <= tm.tm_sec)
> > > +			mdelay(WAIT_TIME_MS);
> > > +	} while (WAIT_AFTER <= tm.tm_sec);
> > 
> > This hardcoded wait for rollover doesn't look good. I see some
> > helper functions in rtc-lib.c which probably could be used for
> > converting the current time to elapsed seconds, add the delay and
> > then convert it back to the time to be programmed in RTC without
> > worrying about rollover. Why not use that?
> 
> I am not aware of those APIs, can you point some?

I have gone through rtc-lib.c, these are the API's I am seeing
in the library

1. rtc_time_to_tm: Convert seconds since 01-01-1970 00:00:00 to
Gregorian date
2. rtc_tm_to_time: Convert Gregorian date to seconds since
01-01-1970 00:00:00

Steps I followed:-
================
1: unsigned long time;
2: omap_rtc_read_time(NULL, &tm);
3: rtc_tm_to_time(tm, &time);
4: pr_info("Time 1 %lu\n", time);
5: time += 2; /* (2sec) */
6: rtc_time_to_tm(time, tm);
7: rtc_tm_to_time(tm, &time); /* Only for printing time value */
8: pr_info("Time 2 %lu\n", time); 

With the above steps I am seeing completely two different time
values at step4 and step8

Thanks
AnilKumar
AnilKumar Chimata Nov. 16, 2012, 10:47 a.m. UTC | #13
On Fri, Nov 16, 2012 at 11:43:36, AnilKumar, Chimata wrote:
> On Mon, Nov 12, 2012 at 15:17:48, AnilKumar, Chimata wrote:
> > On Tue, Nov 06, 2012 at 11:15:34, Bedia, Vaibhav wrote:
> > > On Mon, Nov 05, 2012 at 15:12:27, AnilKumar, Chimata wrote:
> > > [...]
> > > >  
> > > > +#define SHUTDOWN_TIME_SEC		2
> > > > +#define SECS_IN_MIN			60
> > > > +#define WAIT_AFTER			(SECS_IN_MIN - SHUTDOWN_TIME_SEC)
> > > > +#define WAIT_TIME_MS			(SHUTDOWN_TIME_SEC * 1000)
> > > > +
> > > >  static void __iomem	*rtc_base;
> > > >  
> > > [...]
> > > > +
> > > > +	/* Wait few seconds instead of rollover */
> > > > +	do {
> > > > +		omap_rtc_read_time(NULL, &tm);
> > > > +		if (WAIT_AFTER <= tm.tm_sec)
> > > > +			mdelay(WAIT_TIME_MS);
> > > > +	} while (WAIT_AFTER <= tm.tm_sec);
> > > 
> > > This hardcoded wait for rollover doesn't look good. I see some
> > > helper functions in rtc-lib.c which probably could be used for
> > > converting the current time to elapsed seconds, add the delay and
> > > then convert it back to the time to be programmed in RTC without
> > > worrying about rollover. Why not use that?
> > 
> > I am not aware of those APIs, can you point some?
> 
> I have gone through rtc-lib.c, these are the API's I am seeing
> in the library
> 
> 1. rtc_time_to_tm: Convert seconds since 01-01-1970 00:00:00 to
> Gregorian date
> 2. rtc_tm_to_time: Convert Gregorian date to seconds since
> 01-01-1970 00:00:00
> 
> Steps I followed:-
> ================
> 1: unsigned long time;
> 2: omap_rtc_read_time(NULL, &tm);
> 3: rtc_tm_to_time(tm, &time);
> 4: pr_info("Time 1 %lu\n", time);
> 5: time += 2; /* (2sec) */
> 6: rtc_time_to_tm(time, tm);
> 7: rtc_tm_to_time(tm, &time); /* Only for printing time value */
> 8: pr_info("Time 2 %lu\n", time); 
> 
> With the above steps I am seeing completely two different time
> values at step4 and step8
> 

Nevermind it working now.

Thanks
AnilKumar
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
index b47aa41..8d9f4f9 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
@@ -6,6 +6,10 @@  Required properties:
 - interrupts: rtc timer, alarm interrupts in order
 - interrupt-parent: phandle for the interrupt controller
 
+Optional properties:
+- ti,system-power-controller: Telling whether or not rtc is controlling
+  the system power.
+
 Example:
 
 rtc@1c23000 {
@@ -14,4 +18,5 @@  rtc@1c23000 {
 	interrupts = <19
 		      19>;
 	interrupt-parent = <&intc>;
+	ti,system-power-controller;
 };
diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index 6009714..2d90170 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c
@@ -72,6 +72,14 @@ 
 #define OMAP_RTC_KICK0_REG		0x6c
 #define OMAP_RTC_KICK1_REG		0x70
 
+#define OMAP_RTC_ALARM2_SECONDS_REG	0x80
+#define OMAP_RTC_ALARM2_MINUTES_REG	0x84
+#define OMAP_RTC_ALARM2_HOURS_REG	0x88
+#define OMAP_RTC_ALARM2_DAYS_REG	0x8c
+#define OMAP_RTC_ALARM2_MONTHS_REG	0x90
+#define OMAP_RTC_ALARM2_YEARS_REG	0x94
+#define OMAP_RTC_PMIC_REG		0x98
+
 /* OMAP_RTC_CTRL_REG bit fields: */
 #define OMAP_RTC_CTRL_SPLIT		(1<<7)
 #define OMAP_RTC_CTRL_DISABLE		(1<<6)
@@ -93,15 +101,24 @@ 
 #define OMAP_RTC_STATUS_BUSY            (1<<0)
 
 /* OMAP_RTC_INTERRUPTS_REG bit fields: */
+#define OMAP_RTC_INTERRUPTS_IT_ALARM2   (1<<4)
 #define OMAP_RTC_INTERRUPTS_IT_ALARM    (1<<3)
 #define OMAP_RTC_INTERRUPTS_IT_TIMER    (1<<2)
 
+/* OMAP_RTC_PMIC_REG bit fields: */
+#define OMAP_RTC_PMIC_POWER_EN_EN       (1<<16)
+
 /* OMAP_RTC_KICKER values */
 #define	KICK0_VALUE			0x83e70b13
 #define	KICK1_VALUE			0x95a4f1e0
 
 #define	OMAP_RTC_HAS_KICKER		0x1
 
+#define SHUTDOWN_TIME_SEC		2
+#define SECS_IN_MIN			60
+#define WAIT_AFTER			(SECS_IN_MIN - SHUTDOWN_TIME_SEC)
+#define WAIT_TIME_MS			(SHUTDOWN_TIME_SEC * 1000)
+
 static void __iomem	*rtc_base;
 
 #define rtc_read(addr)		readb(rtc_base + (addr))
@@ -290,6 +307,58 @@  static int omap_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
 	return 0;
 }
 
+/*
+ * rtc_power_off: Set the pmic power off sequence. The RTC generates
+ * pmic_pwr_enable control, which can be used to control an external
+ * PMIC.
+ */
+static void rtc_power_off(void)
+{
+	u32 val;
+	struct rtc_time tm;
+	spinlock_t lock;
+	unsigned long flags;
+
+	spin_lock_init(&lock);
+
+	/* Set PMIC power enable */
+	val = readl(rtc_base + OMAP_RTC_PMIC_REG);
+	writel(val | OMAP_RTC_PMIC_POWER_EN_EN, rtc_base + OMAP_RTC_PMIC_REG);
+
+	/* Wait few seconds instead of rollover */
+	do {
+		omap_rtc_read_time(NULL, &tm);
+		if (WAIT_AFTER <= tm.tm_sec)
+			mdelay(WAIT_TIME_MS);
+	} while (WAIT_AFTER <= tm.tm_sec);
+
+	/* Add shutdown time to the current value */
+	tm.tm_sec += SHUTDOWN_TIME_SEC;
+
+	if (tm2bcd(&tm) < 0)
+		return;
+
+	pr_info("System will go to power_off state in approx. %d secs\n",
+			SHUTDOWN_TIME_SEC);
+
+	/* Set the ALARM2 time */
+	rtc_write(tm.tm_sec, OMAP_RTC_ALARM2_SECONDS_REG);
+	rtc_write(tm.tm_min, OMAP_RTC_ALARM2_MINUTES_REG);
+	rtc_write(tm.tm_hour, OMAP_RTC_ALARM2_HOURS_REG);
+	rtc_write(tm.tm_mday, OMAP_RTC_ALARM2_DAYS_REG);
+	rtc_write(tm.tm_mon, OMAP_RTC_ALARM2_MONTHS_REG);
+	rtc_write(tm.tm_year, OMAP_RTC_ALARM2_YEARS_REG);
+
+	/* Enable alarm2 interrupt */
+	val = readl(rtc_base + OMAP_RTC_INTERRUPTS_REG);
+	writel(val | OMAP_RTC_INTERRUPTS_IT_ALARM2,
+				rtc_base + OMAP_RTC_INTERRUPTS_REG);
+
+	/* Do not allow to execute any other task */
+	spin_lock_irqsave(&lock, flags);
+	while (1);
+}
+
 static struct rtc_class_ops omap_rtc_ops = {
 	.read_time	= omap_rtc_read_time,
 	.set_time	= omap_rtc_set_time,
@@ -327,12 +396,16 @@  static int __init omap_rtc_probe(struct platform_device *pdev)
 	struct resource		*res, *mem;
 	struct rtc_device	*rtc;
 	u8			reg, new_ctrl;
+	bool			pm_off = false;
 	const struct platform_device_id *id_entry;
 	const struct of_device_id *of_id;
 
 	of_id = of_match_device(omap_rtc_of_match, &pdev->dev);
-	if (of_id)
+	if (of_id) {
 		pdev->id_entry = of_id->data;
+		pm_off = of_property_read_bool(pdev->dev.of_node,
+					"ti,system-power-controller");
+	}
 
 	omap_rtc_timer = platform_get_irq(pdev, 0);
 	if (omap_rtc_timer <= 0) {
@@ -385,6 +458,10 @@  static int __init omap_rtc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, rtc);
 	dev_set_drvdata(&rtc->dev, mem);
 
+	/* RTC power off */
+	if (pm_off && !pm_power_off)
+		pm_power_off = rtc_power_off;
+
 	/* clear pending irqs, and set 1/second periodic,
 	 * which we'll use instead of update irqs
 	 */