| Message ID | 1372412109-986-4-git-send-email-gururaja.hebbar@ti.com |
|---|---|
| State | Superseded |
| Headers | show |
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
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
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
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
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
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
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
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
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 --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
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(-)