diff mbox

[3/4] rtc: omap: add rtc wakeup support to alarm events

Message ID 1372412109-986-4-git-send-email-gururaja.hebbar@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hebbar, Gururaja June 28, 2013, 9:35 a.m. UTC
On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
is available to enable Alarm Wakeup feature. This register needs to be
properly handled for the rtcwake to work properly.

Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
compatibility node.

Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Rob Landley <rob@landley.net>
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Kevin Hilman <khilman@linaro.org>
Cc: Alessandro Zummo <a.zummo@towertech.it>
Cc: rtc-linux@googlegroups.com
Cc: devicetree-discuss@lists.ozlabs.org
Cc: linux-doc@vger.kernel.org
---
:100644 100644 b47aa41... 5a0f02d... M	Documentation/devicetree/bindings/rtc/rtc-omap.txt
:100644 100644 761919d... 666b0c2... M	drivers/rtc/rtc-omap.c
 Documentation/devicetree/bindings/rtc/rtc-omap.txt |    6 ++-
 drivers/rtc/rtc-omap.c                             |   56 +++++++++++++++++---
 2 files changed, 54 insertions(+), 8 deletions(-)

Comments

Kevin Hilman July 2, 2013, 12:15 a.m. UTC | #1
Hebbar Gururaja <gururaja.hebbar@ti.com> writes:

> On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
> is available to enable Alarm Wakeup feature. This register needs to be
> properly handled for the rtcwake to work properly.
>
> Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
> compatibility node.
>
> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Rob Landley <rob@landley.net>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Kevin Hilman <khilman@linaro.org>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: rtc-linux@googlegroups.com
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: linux-doc@vger.kernel.org

Acked-by: Kevin Hilman <khilman@linaro.org>

...with a minor nit below...

> ---
> :100644 100644 b47aa41... 5a0f02d... M	Documentation/devicetree/bindings/rtc/rtc-omap.txt
> :100644 100644 761919d... 666b0c2... M	drivers/rtc/rtc-omap.c
>  Documentation/devicetree/bindings/rtc/rtc-omap.txt |    6 ++-
>  drivers/rtc/rtc-omap.c                             |   56 +++++++++++++++++---
>  2 files changed, 54 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> index b47aa41..5a0f02d 100644
> --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> @@ -1,7 +1,11 @@
>  TI Real Time Clock
>  
>  Required properties:
> -- compatible: "ti,da830-rtc"
> +- compatible:
> +	- "ti,da830-rtc"  - for RTC IP used similar to that on DA8xx SoC family.
> +	- "ti,am3352-rtc" - for RTC IP used similar to that on AM335x SoC family.
> +			    This RTC IP has special WAKE-EN Register to enable
> +			    Wakeup generation for event Alarm.
>  - reg: Address range of rtc register set
>  - interrupts: rtc timer, alarm interrupts in order
>  - interrupt-parent: phandle for the interrupt controller
> diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> index 761919d..666b0c2 100644
> --- a/drivers/rtc/rtc-omap.c
> +++ b/drivers/rtc/rtc-omap.c
> @@ -72,6 +72,8 @@
>  #define OMAP_RTC_KICK0_REG		0x6c
>  #define OMAP_RTC_KICK1_REG		0x70
>  
> +#define OMAP_RTC_IRQWAKEEN		0x7C
> +

nit: letters in hex numbers are usually lower-case.


Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hebbar, Gururaja July 2, 2013, 5:20 a.m. UTC | #2
On Tue, Jul 02, 2013 at 05:45:01, Kevin Hilman wrote:
> Hebbar Gururaja <gururaja.hebbar@ti.com> writes:
> 
> > On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
> > is available to enable Alarm Wakeup feature. This register needs to be
> > properly handled for the rtcwake to work properly.
> >
> > Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
> > compatibility node.
> >
> > Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> > Cc: Grant Likely <grant.likely@linaro.org>
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Rob Landley <rob@landley.net>
> > Cc: Sekhar Nori <nsekhar@ti.com>
> > Cc: Kevin Hilman <khilman@linaro.org>
> > Cc: Alessandro Zummo <a.zummo@towertech.it>
> > Cc: rtc-linux@googlegroups.com
> > Cc: devicetree-discuss@lists.ozlabs.org
> > Cc: linux-doc@vger.kernel.org
> 
> Acked-by: Kevin Hilman <khilman@linaro.org>
> 
> ...with a minor nit below...
> 
> > ---
> > :100644 100644 b47aa41... 5a0f02d... M	Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > :100644 100644 761919d... 666b0c2... M	drivers/rtc/rtc-omap.c
> >  Documentation/devicetree/bindings/rtc/rtc-omap.txt |    6 ++-
> >  drivers/rtc/rtc-omap.c                             |   56 +++++++++++++++++---
> >  2 files changed, 54 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > index b47aa41..5a0f02d 100644
> > --- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > +++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
> > @@ -1,7 +1,11 @@
> >  TI Real Time Clock
> >  
> >  Required properties:
> > -- compatible: "ti,da830-rtc"
> > +- compatible:
> > +	- "ti,da830-rtc"  - for RTC IP used similar to that on DA8xx SoC family.
> > +	- "ti,am3352-rtc" - for RTC IP used similar to that on AM335x SoC family.
> > +			    This RTC IP has special WAKE-EN Register to enable
> > +			    Wakeup generation for event Alarm.
> >  - reg: Address range of rtc register set
> >  - interrupts: rtc timer, alarm interrupts in order
> >  - interrupt-parent: phandle for the interrupt controller
> > diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
> > index 761919d..666b0c2 100644
> > --- a/drivers/rtc/rtc-omap.c
> > +++ b/drivers/rtc/rtc-omap.c
> > @@ -72,6 +72,8 @@
> >  #define OMAP_RTC_KICK0_REG		0x6c
> >  #define OMAP_RTC_KICK1_REG		0x70
> >  
> > +#define OMAP_RTC_IRQWAKEEN		0x7C
> > +
> 
> nit: letters in hex numbers are usually lower-case.

Thanks for the review. V2 will soon follow.

> 
> 
> Kevin
> 


Regards, 
Gururaja
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sekhar Nori July 2, 2013, 6:02 a.m. UTC | #3
On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
> On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
> is available to enable Alarm Wakeup feature. This register needs to be
> properly handled for the rtcwake to work properly.
> 
> Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
> compatibility node.
> 
> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Rob Landley <rob@landley.net>
> Cc: Sekhar Nori <nsekhar@ti.com>
> Cc: Kevin Hilman <khilman@linaro.org>
> Cc: Alessandro Zummo <a.zummo@towertech.it>
> Cc: rtc-linux@googlegroups.com
> Cc: devicetree-discuss@lists.ozlabs.org
> Cc: linux-doc@vger.kernel.org
> ---

[...]

> -#define	OMAP_RTC_DATA_DA830_IDX	1
> +#define	OMAP_RTC_DATA_DA830_IDX		1
> +#define	OMAP_RTC_DATA_AM335X_IDX	2
>  
>  static struct platform_device_id omap_rtc_devtype[] = {
>  	{
> @@ -309,6 +321,9 @@ static struct platform_device_id omap_rtc_devtype[] = {
>  	}, {
>  		.name	= "da830-rtc",
>  		.driver_data = OMAP_RTC_HAS_KICKER,
> +	}, {
> +		.name	= "am335x-rtc",

may be use am3352-rtc here just to keep the platform device name and of
compatible in sync.

> +		.driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,
>  	},
>  	{},

It is better to use the index defined above in the static initialization
so they remain in sync.

	...
	[OMAP_RTC_DATA_DA830_IDX] = {
		.name	= "da830-rtc",
		.driver_data = OMAP_RTC_HAS_KICKER,
	},
	...

>  };
> @@ -318,6 +333,9 @@ static const struct of_device_id omap_rtc_of_match[] = {
>  	{	.compatible	= "ti,da830-rtc",
>  		.data		= &omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX],
>  	},
> +	{	.compatible	= "ti,am3352-rtc",
> +		.data		= &omap_rtc_devtype[OMAP_RTC_DATA_AM335X_IDX],
> +	},
>  	{},
>  };
>  MODULE_DEVICE_TABLE(of, omap_rtc_of_match);

Apart from these minor issues, the patch looks good to me.

Acked-by: Sekhar Nori <nsekhar@ti.com>

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hebbar, Gururaja July 2, 2013, 6:04 a.m. UTC | #4
On Tue, Jul 02, 2013 at 11:32:34, Nori, Sekhar wrote:
> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:

> > On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)

> > is available to enable Alarm Wakeup feature. This register needs to be

> > properly handled for the rtcwake to work properly.

> > 

> > Platforms using such IP should set "ti,am3352-rtc" in rtc device dt

> > compatibility node.

> > 

> > Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>

> > Cc: Grant Likely <grant.likely@linaro.org>

> > Cc: Rob Herring <rob.herring@calxeda.com>

> > Cc: Rob Landley <rob@landley.net>

> > Cc: Sekhar Nori <nsekhar@ti.com>

> > Cc: Kevin Hilman <khilman@linaro.org>

> > Cc: Alessandro Zummo <a.zummo@towertech.it>

> > Cc: rtc-linux@googlegroups.com

> > Cc: devicetree-discuss@lists.ozlabs.org

> > Cc: linux-doc@vger.kernel.org

> > ---

> 

> [...]

> 

> > -#define	OMAP_RTC_DATA_DA830_IDX	1

> > +#define	OMAP_RTC_DATA_DA830_IDX		1

> > +#define	OMAP_RTC_DATA_AM335X_IDX	2

> >  

> >  static struct platform_device_id omap_rtc_devtype[] = {

> >  	{

> > @@ -309,6 +321,9 @@ static struct platform_device_id omap_rtc_devtype[] = {

> >  	}, {

> >  		.name	= "da830-rtc",

> >  		.driver_data = OMAP_RTC_HAS_KICKER,

> > +	}, {

> > +		.name	= "am335x-rtc",

> 

> may be use am3352-rtc here just to keep the platform device name and of

> compatible in sync.


Correct. I will update the same in v2.

> 

> > +		.driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,

> >  	},

> >  	{},

> 

> It is better to use the index defined above in the static initialization

> so they remain in sync.


Sorry. I didn’t get this.

> 

> 	...

> 	[OMAP_RTC_DATA_DA830_IDX] = {

> 		.name	= "da830-rtc",

> 		.driver_data = OMAP_RTC_HAS_KICKER,

> 	},

> 	...

> 

> >  };

> > @@ -318,6 +333,9 @@ static const struct of_device_id omap_rtc_of_match[] = {

> >  	{	.compatible	= "ti,da830-rtc",

> >  		.data		= &omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX],

> >  	},

> > +	{	.compatible	= "ti,am3352-rtc",

> > +		.data		= &omap_rtc_devtype[OMAP_RTC_DATA_AM335X_IDX],

> > +	},

> >  	{},

> >  };

> >  MODULE_DEVICE_TABLE(of, omap_rtc_of_match);

> 

> Apart from these minor issues, the patch looks good to me.

> 

> Acked-by: Sekhar Nori <nsekhar@ti.com>

> 

> Thanks,

> Sekhar

> 



Regards, 
Gururaja
Sekhar Nori July 2, 2013, 6:09 a.m. UTC | #5
On 7/2/2013 11:34 AM, Hebbar, Gururaja wrote:
> On Tue, Jul 02, 2013 at 11:32:34, Nori, Sekhar wrote:
>> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
>>> On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
>>> is available to enable Alarm Wakeup feature. This register needs to be
>>> properly handled for the rtcwake to work properly.
>>>
>>> Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
>>> compatibility node.
>>>
>>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
>>> Cc: Grant Likely <grant.likely@linaro.org>
>>> Cc: Rob Herring <rob.herring@calxeda.com>
>>> Cc: Rob Landley <rob@landley.net>
>>> Cc: Sekhar Nori <nsekhar@ti.com>
>>> Cc: Kevin Hilman <khilman@linaro.org>
>>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>>> Cc: rtc-linux@googlegroups.com
>>> Cc: devicetree-discuss@lists.ozlabs.org
>>> Cc: linux-doc@vger.kernel.org
>>> ---
>>
>> [...]
>>
>>> -#define	OMAP_RTC_DATA_DA830_IDX	1
>>> +#define	OMAP_RTC_DATA_DA830_IDX		1
>>> +#define	OMAP_RTC_DATA_AM335X_IDX	2
>>>  
>>>  static struct platform_device_id omap_rtc_devtype[] = {
>>>  	{
>>> @@ -309,6 +321,9 @@ static struct platform_device_id omap_rtc_devtype[] = {
>>>  	}, {
>>>  		.name	= "da830-rtc",
>>>  		.driver_data = OMAP_RTC_HAS_KICKER,
>>> +	}, {
>>> +		.name	= "am335x-rtc",
>>
>> may be use am3352-rtc here just to keep the platform device name and of
>> compatible in sync.
> 
> Correct. I will update the same in v2.
> 
>>
>>> +		.driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,
>>>  	},
>>>  	{},
>>
>> It is better to use the index defined above in the static initialization
>> so they remain in sync.
> 
> Sorry. I didn’t get this.
> 

See example below I provided. If its still not clear, let me know what
is not clear.

>> 	...
>> 	[OMAP_RTC_DATA_DA830_IDX] = {
>> 		.name	= "da830-rtc",
>> 		.driver_data = OMAP_RTC_HAS_KICKER,
>> 	},

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hebbar, Gururaja July 2, 2013, 6:11 a.m. UTC | #6
On Tue, Jul 02, 2013 at 11:39:28, Nori, Sekhar wrote:
> On 7/2/2013 11:34 AM, Hebbar, Gururaja wrote:

> > On Tue, Jul 02, 2013 at 11:32:34, Nori, Sekhar wrote:

> >> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:

> >>> On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)

> >>> is available to enable Alarm Wakeup feature. This register needs to be

> >>> properly handled for the rtcwake to work properly.

> >>>

> >>> Platforms using such IP should set "ti,am3352-rtc" in rtc device dt

> >>> compatibility node.

> >>>

> >>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>

> >>> Cc: Grant Likely <grant.likely@linaro.org>

> >>> Cc: Rob Herring <rob.herring@calxeda.com>

> >>> Cc: Rob Landley <rob@landley.net>

> >>> Cc: Sekhar Nori <nsekhar@ti.com>

> >>> Cc: Kevin Hilman <khilman@linaro.org>

> >>> Cc: Alessandro Zummo <a.zummo@towertech.it>

> >>> Cc: rtc-linux@googlegroups.com

> >>> Cc: devicetree-discuss@lists.ozlabs.org

> >>> Cc: linux-doc@vger.kernel.org

> >>> ---

> >>

> >> [...]

> >>

> >>> -#define	OMAP_RTC_DATA_DA830_IDX	1

> >>> +#define	OMAP_RTC_DATA_DA830_IDX		1

> >>> +#define	OMAP_RTC_DATA_AM335X_IDX	2

> >>>  

> >>>  static struct platform_device_id omap_rtc_devtype[] = {

> >>>  	{

> >>> @@ -309,6 +321,9 @@ static struct platform_device_id omap_rtc_devtype[] = {

> >>>  	}, {

> >>>  		.name	= "da830-rtc",

> >>>  		.driver_data = OMAP_RTC_HAS_KICKER,

> >>> +	}, {

> >>> +		.name	= "am335x-rtc",

> >>

> >> may be use am3352-rtc here just to keep the platform device name and of

> >> compatible in sync.

> > 

> > Correct. I will update the same in v2.

> > 

> >>

> >>> +		.driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,

> >>>  	},

> >>>  	{},

> >>

> >> It is better to use the index defined above in the static initialization

> >> so they remain in sync.

> > 

> > Sorry. I didn’t get this.

> > 

> 

> See example below I provided. If its still not clear, let me know what

> is not clear.

> 

> >> 	...

> >> 	[OMAP_RTC_DATA_DA830_IDX] = {

> >> 		.name	= "da830-rtc",

> >> 		.driver_data = OMAP_RTC_HAS_KICKER,

> >> 	},


Thanks for the clarification. In this case will it ok if I update the previous
member also.

> 

> Thanks,

> Sekhar

> 



Regards, 
Gururaja
Sekhar Nori July 2, 2013, 6:16 a.m. UTC | #7
On 7/2/2013 11:41 AM, Hebbar, Gururaja wrote:
> On Tue, Jul 02, 2013 at 11:39:28, Nori, Sekhar wrote:
>> On 7/2/2013 11:34 AM, Hebbar, Gururaja wrote:
>>> On Tue, Jul 02, 2013 at 11:32:34, Nori, Sekhar wrote:
>>>> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:
>>>>> On some platforms (like AM33xx), a special register (RTC_IRQWAKEEN)
>>>>> is available to enable Alarm Wakeup feature. This register needs to be
>>>>> properly handled for the rtcwake to work properly.
>>>>>
>>>>> Platforms using such IP should set "ti,am3352-rtc" in rtc device dt
>>>>> compatibility node.
>>>>>
>>>>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com>
>>>>> Cc: Grant Likely <grant.likely@linaro.org>
>>>>> Cc: Rob Herring <rob.herring@calxeda.com>
>>>>> Cc: Rob Landley <rob@landley.net>
>>>>> Cc: Sekhar Nori <nsekhar@ti.com>
>>>>> Cc: Kevin Hilman <khilman@linaro.org>
>>>>> Cc: Alessandro Zummo <a.zummo@towertech.it>
>>>>> Cc: rtc-linux@googlegroups.com
>>>>> Cc: devicetree-discuss@lists.ozlabs.org
>>>>> Cc: linux-doc@vger.kernel.org
>>>>> ---
>>>>
>>>> [...]
>>>>
>>>>> -#define	OMAP_RTC_DATA_DA830_IDX	1
>>>>> +#define	OMAP_RTC_DATA_DA830_IDX		1
>>>>> +#define	OMAP_RTC_DATA_AM335X_IDX	2
>>>>>  
>>>>>  static struct platform_device_id omap_rtc_devtype[] = {
>>>>>  	{
>>>>> @@ -309,6 +321,9 @@ static struct platform_device_id omap_rtc_devtype[] = {
>>>>>  	}, {
>>>>>  		.name	= "da830-rtc",
>>>>>  		.driver_data = OMAP_RTC_HAS_KICKER,
>>>>> +	}, {
>>>>> +		.name	= "am335x-rtc",
>>>>
>>>> may be use am3352-rtc here just to keep the platform device name and of
>>>> compatible in sync.
>>>
>>> Correct. I will update the same in v2.
>>>
>>>>
>>>>> +		.driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,
>>>>>  	},
>>>>>  	{},
>>>>
>>>> It is better to use the index defined above in the static initialization
>>>> so they remain in sync.
>>>
>>> Sorry. I didn’t get this.
>>>
>>
>> See example below I provided. If its still not clear, let me know what
>> is not clear.
>>
>>>> 	...
>>>> 	[OMAP_RTC_DATA_DA830_IDX] = {
>>>> 		.name	= "da830-rtc",
>>>> 		.driver_data = OMAP_RTC_HAS_KICKER,
>>>> 	},
> 
> Thanks for the clarification. In this case will it ok if I update the previous
> member also.

You dont really reference [0] in omap_rtc_of_match[] so even if you
leave it as-is, that's fine with me. I am mostly concerned with the
index definitions and initialization order being out of sync and that's
really not an issue with [0].

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hebbar, Gururaja July 3, 2013, 5:03 a.m. UTC | #8
Hi all,

Kindly ignore this message. It was sent in wrong format.

Sorry for the noise

Regards, 
Gururaja

On Wed, Jul 03, 2013 at 10:26:57, Hebbar, Gururaja wrote:
> Below is the code snippet I was referring to

>  

>  

> From drivers/rtc/rtc-omap.c

>  

> static struct platform_device_id omap_rtc_devtype[] = {

>       {

>             .name = DRIVER_NAME,

>       },

>       [OMAP_RTC_DATA_AM3352_IDX] = {

>             .name = "am3352-rtc",

>             .driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,

>       },

>       [OMAP_RTC_DATA_DA830_IDX] = {

>             .name = "da830-rtc",

>             .driver_data = OMAP_RTC_HAS_KICKER,

>      },

>       {},

> };

> MODULE_DEVICE_TABLE(platform, omap_rtc_devtype);

>  

> static const struct of_device_id omap_rtc_of_match[] = {

>       {     .compatible = "ti,da830-rtc",

>             .data       = &omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX],

>       },

>       {     .compatible = "ti,am3352-rtc",

>             .data       = &omap_rtc_devtype[OMAP_RTC_DATA_AM3352_IDX],

>       },

>       {},

> };

> MODULE_DEVICE_TABLE(of, omap_rtc_of_match);

>  

>  

> From arch/arm/boot/dts/am33xx.dtsi

>  

>             rtc@44e3e000 {

>                   compatible = "ti,da830-rtc", "ti,am3352-rtc";

>                   reg = <0x44e3e000 0x1000>;

>                   interrupts = <75

>                               76>;

>                   ti,hwmods = "rtc";

>             };

>  

>  

> As seen from above snippet, 2 compatible items are specified for

> compatible dt property (ti,da830-rtc" & "ti,am3352-rtc”)

> These are the same compatibles that are mentioned in the of_device_id

> structure inside rtc-omap driver.

>  

> What I observed is, if we mention both compatible in the .dtsi file that

> are under one single of_device_id structure, the first match from the

> of_device_id structure is considered (ti,da830-rtc in above case)

>  

> To confirm, I switched the 2 compatible inside of_device_id structure as

> below

>  

>  

> static const struct of_device_id omap_rtc_of_match[] = {

>       {     .compatible = "ti,am3352-rtc",

>             .data       = &omap_rtc_devtype[OMAP_RTC_DATA_AM3352_IDX],

>       },

>       {     .compatible = "ti,da830-rtc",

>             .data       = &omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX],

>       },

>       {},

> };

>  

> In this case, the first match from compatible field was chosen

> (ti,am3352-rtc now).

>  

>  

> Hope this is clear.

>  

> Kindly let me know when you are free to discuss.

>  

>  

> Regards

> Gururaja

>  

> > -----Original Message-----

> > From: Nori, Sekhar

> > Sent: Tuesday, July 02, 2013 11:47 AM

> > To: Hebbar, Gururaja

> > Cc: khilman@linaro.org; tony@atomide.com; Cousson, Benoit; linux-

> > omap@vger.kernel.org; devicetree-discuss@lists.ozlabs.org; linux-

> > kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;

> > davinci-linux-open-source@linux.davincidsp.com; Bedia, Vaibhav;

> > Rajashekhara, Sudhakar; Grant Likely; Rob Herring; Rob Landley;

> > Alessandro Zummo; rtc-linux@googlegroups.com; linux-

> > doc@vger.kernel.org

> > Subject: Re: [PATCH 3/4] rtc: omap: add rtc wakeup support to

> > alarm events

> > 

> > 

> > 

> > On 7/2/2013 11:41 AM, Hebbar, Gururaja wrote:

> > > On Tue, Jul 02, 2013 at 11:39:28, Nori, Sekhar wrote:

> > >> On 7/2/2013 11:34 AM, Hebbar, Gururaja wrote:

> > >>> On Tue, Jul 02, 2013 at 11:32:34, Nori, Sekhar wrote:

> > >>>> On 6/28/2013 3:05 PM, Hebbar Gururaja wrote:

> > >>>>> On some platforms (like AM33xx), a special register

> > (RTC_IRQWAKEEN)

> > >>>>> is available to enable Alarm Wakeup feature. This register

> > needs to be

> > >>>>> properly handled for the rtcwake to work properly.

> > >>>>>

> > >>>>> Platforms using such IP should set "ti,am3352-rtc" in rtc

> > device dt

> > >>>>> compatibility node.

> > >>>>>

> > >>>>> Signed-off-by: Hebbar Gururaja <gururaja.hebbar@ti.com

> <mailto:gururaja.hebbar@ti.com> >

> > >>>>> Cc: Grant Likely <grant.likely@linaro.org

> <mailto:grant.likely@linaro.org> >

> > >>>>> Cc: Rob Herring <rob.herring@calxeda.com

> <mailto:rob.herring@calxeda.com> >

> > >>>>> Cc: Rob Landley <rob@landley.net <mailto:rob@landley.net> >

> > >>>>> Cc: Sekhar Nori <nsekhar@ti.com <mailto:nsekhar@ti.com> >

> > >>>>> Cc: Kevin Hilman <khilman@linaro.org <mailto:khilman@linaro.org>

> >

> > >>>>> Cc: Alessandro Zummo <a.zummo@towertech.it

> <mailto:a.zummo@towertech.it> >

> > >>>>> Cc: rtc-linux@googlegroups.com

> <mailto:rtc-linux@googlegroups.com> 

> > >>>>> Cc: devicetree-discuss@lists.ozlabs.org

> <mailto:devicetree-discuss@lists.ozlabs.org> 

> > >>>>> Cc: linux-doc@vger.kernel.org <mailto:linux-doc@vger.kernel.org>

> > >>>>> ---

> > >>>>

> > >>>> [...]

> > >>>>

> > >>>>> -#define  OMAP_RTC_DATA_DA830_IDX 1

> > >>>>> +#define  OMAP_RTC_DATA_DA830_IDX       1

> > >>>>> +#define  OMAP_RTC_DATA_AM335X_IDX      2

> > >>>>>

> > >>>>>  static struct platform_device_id omap_rtc_devtype[] = {

> > >>>>>     {

> > >>>>> @@ -309,6 +321,9 @@ static struct platform_device_id

> > omap_rtc_devtype[] = {

> > >>>>>     }, {

> > >>>>>           .name = "da830-rtc",

> > >>>>>           .driver_data = OMAP_RTC_HAS_KICKER,

> > >>>>> +   }, {

> > >>>>> +         .name = "am335x-rtc",

> > >>>>

> > >>>> may be use am3352-rtc here just to keep the platform device

> > name and of

> > >>>> compatible in sync.

> > >>>

> > >>> Correct. I will update the same in v2.

> > >>>

> > >>>>

> > >>>>> +         .driver_data = OMAP_RTC_HAS_KICKER |

> > OMAP_RTC_HAS_IRQWAKEEN,

> > >>>>>     },

> > >>>>>     {},

> > >>>>

> > >>>> It is better to use the index defined above in the static

> > initialization

> > >>>> so they remain in sync.

> > >>>

> > >>> Sorry. I didn’t get this.

> > >>>

> > >>

> > >> See example below I provided. If its still not clear, let me

> > know what

> > >> is not clear.

> > >>

> > >>>>      ...

> > >>>>      [OMAP_RTC_DATA_DA830_IDX] = {

> > >>>>            .name = "da830-rtc",

> > >>>>            .driver_data = OMAP_RTC_HAS_KICKER,

> > >>>>      },

> > >

> > > Thanks for the clarification. In this case will it ok if I

> > update the previous

> > > member also.

> > 

> > You dont really reference [0] in omap_rtc_of_match[] so even if

> > you

> > leave it as-is, that's fine with me. I am mostly concerned with

> > the

> > index definitions and initialization order being out of sync and

> > that's

> > really not an issue with [0].

> > 

> > Thanks,

> > Sekhar

>
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/rtc/rtc-omap.txt b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
index b47aa41..5a0f02d 100644
--- a/Documentation/devicetree/bindings/rtc/rtc-omap.txt
+++ b/Documentation/devicetree/bindings/rtc/rtc-omap.txt
@@ -1,7 +1,11 @@ 
 TI Real Time Clock
 
 Required properties:
-- compatible: "ti,da830-rtc"
+- compatible:
+	- "ti,da830-rtc"  - for RTC IP used similar to that on DA8xx SoC family.
+	- "ti,am3352-rtc" - for RTC IP used similar to that on AM335x SoC family.
+			    This RTC IP has special WAKE-EN Register to enable
+			    Wakeup generation for event Alarm.
 - reg: Address range of rtc register set
 - interrupts: rtc timer, alarm interrupts in order
 - interrupt-parent: phandle for the interrupt controller
diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c
index 761919d..666b0c2 100644
--- a/drivers/rtc/rtc-omap.c
+++ b/drivers/rtc/rtc-omap.c
@@ -72,6 +72,8 @@ 
 #define OMAP_RTC_KICK0_REG		0x6c
 #define OMAP_RTC_KICK1_REG		0x70
 
+#define OMAP_RTC_IRQWAKEEN		0x7C
+
 /* OMAP_RTC_CTRL_REG bit fields: */
 #define OMAP_RTC_CTRL_SPLIT		(1<<7)
 #define OMAP_RTC_CTRL_DISABLE		(1<<6)
@@ -96,12 +98,21 @@ 
 #define OMAP_RTC_INTERRUPTS_IT_ALARM    (1<<3)
 #define OMAP_RTC_INTERRUPTS_IT_TIMER    (1<<2)
 
+/* OMAP_RTC_IRQWAKEEN bit fields: */
+#define OMAP_RTC_IRQWAKEEN_ALARM_WAKEEN    (1<<1)
+
 /* OMAP_RTC_KICKER values */
 #define	KICK0_VALUE			0x83e70b13
 #define	KICK1_VALUE			0x95a4f1e0
 
 #define	OMAP_RTC_HAS_KICKER		0x1
 
+/*
+ * Few RTC IP revisions has special WAKE-EN Register to enable Wakeup
+ * generation for event Alarm.
+ */
+#define	OMAP_RTC_HAS_IRQWAKEEN		0x2
+
 static void __iomem	*rtc_base;
 
 #define rtc_read(addr)		readb(rtc_base + (addr))
@@ -301,7 +312,8 @@  static struct rtc_class_ops omap_rtc_ops = {
 static int omap_rtc_alarm;
 static int omap_rtc_timer;
 
-#define	OMAP_RTC_DATA_DA830_IDX	1
+#define	OMAP_RTC_DATA_DA830_IDX		1
+#define	OMAP_RTC_DATA_AM335X_IDX	2
 
 static struct platform_device_id omap_rtc_devtype[] = {
 	{
@@ -309,6 +321,9 @@  static struct platform_device_id omap_rtc_devtype[] = {
 	}, {
 		.name	= "da830-rtc",
 		.driver_data = OMAP_RTC_HAS_KICKER,
+	}, {
+		.name	= "am335x-rtc",
+		.driver_data = OMAP_RTC_HAS_KICKER | OMAP_RTC_HAS_IRQWAKEEN,
 	},
 	{},
 };
@@ -318,6 +333,9 @@  static const struct of_device_id omap_rtc_of_match[] = {
 	{	.compatible	= "ti,da830-rtc",
 		.data		= &omap_rtc_devtype[OMAP_RTC_DATA_DA830_IDX],
 	},
+	{	.compatible	= "ti,am3352-rtc",
+		.data		= &omap_rtc_devtype[OMAP_RTC_DATA_AM335X_IDX],
+	},
 	{},
 };
 MODULE_DEVICE_TABLE(of, omap_rtc_of_match);
@@ -466,16 +484,28 @@  static u8 irqstat;
 
 static int omap_rtc_suspend(struct device *dev)
 {
+	u8 irqwake_stat;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct platform_device_id *id_entry =
+					platform_get_device_id(pdev);
+
 	irqstat = rtc_read(OMAP_RTC_INTERRUPTS_REG);
 
 	/* FIXME the RTC alarm is not currently acting as a wakeup event
-	 * source, and in fact this enable() call is just saving a flag
-	 * that's never used...
+	 * source on some platforms, and in fact this enable() call is just
+	 * saving a flag that's never used...
 	 */
-	if (device_may_wakeup(dev))
+	if (device_may_wakeup(dev)) {
 		enable_irq_wake(omap_rtc_alarm);
-	else
+
+		if (id_entry->driver_data & OMAP_RTC_HAS_IRQWAKEEN) {
+			irqwake_stat = rtc_read(OMAP_RTC_IRQWAKEEN);
+			irqwake_stat |= OMAP_RTC_IRQWAKEEN_ALARM_WAKEEN;
+			rtc_write(irqwake_stat, OMAP_RTC_IRQWAKEEN);
+		}
+	} else {
 		rtc_write(0, OMAP_RTC_INTERRUPTS_REG);
+	}
 
 	/* Disable the clock/module */
 	pm_runtime_put_sync(dev);
@@ -485,13 +515,25 @@  static int omap_rtc_suspend(struct device *dev)
 
 static int omap_rtc_resume(struct device *dev)
 {
+	u8 irqwake_stat;
+	struct platform_device *pdev = to_platform_device(dev);
+	const struct platform_device_id *id_entry =
+				platform_get_device_id(pdev);
+
 	/* Enable the clock/module so that we can access the registers */
 	pm_runtime_get_sync(dev);
 
-	if (device_may_wakeup(dev))
+	if (device_may_wakeup(dev)) {
 		disable_irq_wake(omap_rtc_alarm);
-	else
+
+		if (id_entry->driver_data & OMAP_RTC_HAS_IRQWAKEEN) {
+			irqwake_stat = rtc_read(OMAP_RTC_IRQWAKEEN);
+			irqwake_stat &= ~OMAP_RTC_IRQWAKEEN_ALARM_WAKEEN;
+			rtc_write(irqwake_stat, OMAP_RTC_IRQWAKEEN);
+		}
+	} else {
 		rtc_write(irqstat, OMAP_RTC_INTERRUPTS_REG);
+	}
 	return 0;
 }
 #endif