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

login
register
mail settings
Submitter Hebbar, Gururaja
Date June 28, 2013, 9:35 a.m.
Message ID <1372412109-986-4-git-send-email-gururaja.hebbar@ti.com>
Download mbox | patch
Permalink /patch/255322/
State New
Headers show

Comments

Hebbar, Gururaja - June 28, 2013, 9:35 a.m.
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(-)
Kevin Hilman - July 2, 2013, 12:15 a.m.
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
Hebbar, Gururaja - July 2, 2013, 5:20 a.m.
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
Sekhar Nori - July 2, 2013, 6:02 a.m.
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
Hebbar, Gururaja - July 2, 2013, 6:04 a.m.
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.
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
Hebbar, Gururaja - July 2, 2013, 6:11 a.m.
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.
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
Hebbar, Gururaja - July 3, 2013, 4:56 a.m.
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
Hebbar, Gururaja - July 3, 2013, 5:03 a.m.
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
>

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