Re: [PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events
Hebbar Gururaja 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 > Cc: Grant Likely > Cc: Rob Herring > Cc: Rob Landley > Cc: Sekhar Nori > Cc: Kevin Hilman > Cc: Alessandro Zummo > Cc: rtc-li...@googlegroups.com > Cc: devicetree-disc...@lists.ozlabs.org > Cc: linux-...@vger.kernel.org Acked-by: Kevin Hilman ...with a minor nit below... > --- > :100644 100644 b47aa41... 5a0f02d... M > Documentation/devicetree/bindings/rtc/rtc-omap.txt > :100644 100644 761919d... 666b0c2... Mdrivers/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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events
On Tue, Jul 02, 2013 at 05:45:01, Kevin Hilman wrote: > Hebbar Gururaja 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 > > Cc: Grant Likely > > Cc: Rob Herring > > Cc: Rob Landley > > Cc: Sekhar Nori > > Cc: Kevin Hilman > > Cc: Alessandro Zummo > > Cc: rtc-li...@googlegroups.com > > Cc: devicetree-disc...@lists.ozlabs.org > > Cc: linux-...@vger.kernel.org > > Acked-by: Kevin Hilman > > ...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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events
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 > Cc: Grant Likely > Cc: Rob Herring > Cc: Rob Landley > Cc: Sekhar Nori > Cc: Kevin Hilman > Cc: Alessandro Zummo > Cc: rtc-li...@googlegroups.com > Cc: devicetree-disc...@lists.ozlabs.org > Cc: linux-...@vger.kernel.org > --- [...] > -#define OMAP_RTC_DATA_DA830_IDX 1 > +#define OMAP_RTC_DATA_DA830_IDX 1 > +#define OMAP_RTC_DATA_AM335X_IDX2 > > 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 Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events
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 > > Cc: Grant Likely > > Cc: Rob Herring > > Cc: Rob Landley > > Cc: Sekhar Nori > > Cc: Kevin Hilman > > Cc: Alessandro Zummo > > Cc: rtc-li...@googlegroups.com > > Cc: devicetree-disc...@lists.ozlabs.org > > Cc: linux-...@vger.kernel.org > > --- > > [...] > > > -#defineOMAP_RTC_DATA_DA830_IDX 1 > > +#defineOMAP_RTC_DATA_DA830_IDX 1 > > +#defineOMAP_RTC_DATA_AM335X_IDX2 > > > > 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 > > Thanks, > Sekhar > Regards, Gururaja
Re: [PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events
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 >>> Cc: Grant Likely >>> Cc: Rob Herring >>> Cc: Rob Landley >>> Cc: Sekhar Nori >>> Cc: Kevin Hilman >>> Cc: Alessandro Zummo >>> Cc: rtc-li...@googlegroups.com >>> Cc: devicetree-disc...@lists.ozlabs.org >>> Cc: linux-...@vger.kernel.org >>> --- >> >> [...] >> >>> -#defineOMAP_RTC_DATA_DA830_IDX 1 >>> +#defineOMAP_RTC_DATA_DA830_IDX 1 >>> +#defineOMAP_RTC_DATA_AM335X_IDX2 >>> >>> 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events
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 > >>> Cc: Grant Likely > >>> Cc: Rob Herring > >>> Cc: Rob Landley > >>> Cc: Sekhar Nori > >>> Cc: Kevin Hilman > >>> Cc: Alessandro Zummo > >>> Cc: rtc-li...@googlegroups.com > >>> Cc: devicetree-disc...@lists.ozlabs.org > >>> Cc: linux-...@vger.kernel.org > >>> --- > >> > >> [...] > >> > >>> -#define OMAP_RTC_DATA_DA830_IDX 1 > >>> +#define OMAP_RTC_DATA_DA830_IDX 1 > >>> +#define OMAP_RTC_DATA_AM335X_IDX2 > >>> > >>> 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
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 > Cc: Grant Likely > Cc: Rob Herring > Cc: Rob Landley > Cc: Sekhar Nori > Cc: Kevin Hilman > Cc: Alessandro Zummo > Cc: rtc-li...@googlegroups.com > Cc: devicetree-disc...@lists.ozlabs.org > Cc: linux-...@vger.kernel.org > --- [...] > -#define OMAP_RTC_DATA_DA830_IDX 1 > +#define OMAP_RTC_DATA_DA830_IDX 1 > +#define OMAP_RTC_DATA_AM335X_IDX2 > > 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 3/4] rtc: omap: add rtc wakeup support to alarm events
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: khil...@linaro.org; t...@atomide.com; Cousson, Benoit; linux- > > o...@vger.kernel.org; devicetree-disc...@lists.ozlabs.org; linux- > > ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > > davinci-linux-open-sou...@linux.davincidsp.com; Bedia, Vaibhav; > > Rajashekhara, Sudhakar; Grant Likely; Rob Herring; Rob Landley; > > Alessandro Zummo; rtc-li...@googlegroups.com; linux- > > d...@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 <mailto:gururaja.heb...@ti.com> > > > >>>>> Cc: Grant Likely <mailto:grant.lik...@linaro.org> > > > >>>>> Cc: Rob Herring <mailto:rob.herr...@calxeda.com> > > > >>>>> Cc: Rob Landley mailto:r...@landley.net> > > > >>>>> Cc: Sekhar Nori mailto:nsek...@ti.com> > > > >>>>