| Message ID | 1425375722-13412-1-git-send-email-j-keerthy@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tuesday 03 March 2015 03:12 PM, Keerthy wrote: > Add external 32k clock feature. The internal clock will be gated during suspend. > Hence make use of the external 32k clock so that rtc is functional accross > suspend/resume. A gentle ping on this. > > Signed-off-by: Keerthy <j-keerthy@ti.com> > --- > > Tested on DRA7-EVM. > > drivers/rtc/rtc-omap.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c > index 8e5851a..4f803ca 100644 > --- a/drivers/rtc/rtc-omap.c > +++ b/drivers/rtc/rtc-omap.c > @@ -107,6 +107,8 @@ > > /* OMAP_RTC_OSC_REG bit fields: */ > #define OMAP_RTC_OSC_32KCLK_EN BIT(6) > +#define OMAP_RTC_OSC_OSC32K_GZ BIT(4) > +#define OMAP_RTC_OSC_EXT_32K BIT(3) > > /* OMAP_RTC_IRQWAKEEN bit fields: */ > #define OMAP_RTC_IRQWAKEEN_ALARM_WAKEEN BIT(1) > @@ -120,6 +122,7 @@ > > struct omap_rtc_device_type { > bool has_32kclk_en; > + bool has_osc_ext_32k; > bool has_kicker; > bool has_irqwakeen; > bool has_pmic_mode; > @@ -446,6 +449,7 @@ static const struct omap_rtc_device_type omap_rtc_default_type = { > > static const struct omap_rtc_device_type omap_rtc_am3352_type = { > .has_32kclk_en = true, > + .has_osc_ext_32k = true, > .has_kicker = true, > .has_irqwakeen = true, > .has_pmic_mode = true, > @@ -543,7 +547,16 @@ static int __init omap_rtc_probe(struct platform_device *pdev) > if (rtc->type->has_32kclk_en) { > reg = rtc_read(rtc, OMAP_RTC_OSC_REG); > rtc_writel(rtc, OMAP_RTC_OSC_REG, > - reg | OMAP_RTC_OSC_32KCLK_EN); > + reg | OMAP_RTC_OSC_32KCLK_EN); > + } > + > + /* Enable External clock as the source */ > + > + if (rtc->type->has_osc_ext_32k) { > + rtc_writel(rtc, OMAP_RTC_OSC_REG, > + (OMAP_RTC_OSC_EXT_32K | > + rtc_read(rtc, OMAP_RTC_OSC_REG)) & > + (~OMAP_RTC_OSC_OSC32K_GZ)); > } > > /* clear old status */ > -- 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
On Tue, 3 Mar 2015 15:12:02 +0530 Keerthy <j-keerthy@ti.com> wrote: > Add external 32k clock feature. The internal clock will be gated during suspend. > Hence make use of the external 32k clock so that rtc is functional accross > suspend/resume. > > ... > > @@ -446,6 +449,7 @@ static const struct omap_rtc_device_type omap_rtc_default_type = { > > static const struct omap_rtc_device_type omap_rtc_am3352_type = { > .has_32kclk_en = true, > + .has_osc_ext_32k = true, > .has_kicker = true, > .has_irqwakeen = true, > .has_pmic_mode = true, > @@ -543,7 +547,16 @@ static int __init omap_rtc_probe(struct platform_device *pdev) > if (rtc->type->has_32kclk_en) { > reg = rtc_read(rtc, OMAP_RTC_OSC_REG); > rtc_writel(rtc, OMAP_RTC_OSC_REG, > - reg | OMAP_RTC_OSC_32KCLK_EN); > + reg | OMAP_RTC_OSC_32KCLK_EN); > + } > + > + /* Enable External clock as the source */ > + > + if (rtc->type->has_osc_ext_32k) { > + rtc_writel(rtc, OMAP_RTC_OSC_REG, > + (OMAP_RTC_OSC_EXT_32K | > + rtc_read(rtc, OMAP_RTC_OSC_REG)) & > + (~OMAP_RTC_OSC_OSC32K_GZ)); > } How do we know that all systems have this external clock and that it works OK? -- 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
Hi Andrew, Apologies for replying late. On Wednesday 25 March 2015 04:29 AM, Andrew Morton wrote: > On Tue, 3 Mar 2015 15:12:02 +0530 Keerthy <j-keerthy@ti.com> wrote: > >> Add external 32k clock feature. The internal clock will be gated during suspend. >> Hence make use of the external 32k clock so that rtc is functional accross >> suspend/resume. >> >> ... >> >> @@ -446,6 +449,7 @@ static const struct omap_rtc_device_type omap_rtc_default_type = { >> >> static const struct omap_rtc_device_type omap_rtc_am3352_type = { >> .has_32kclk_en = true, >> + .has_osc_ext_32k = true, >> .has_kicker = true, >> .has_irqwakeen = true, >> .has_pmic_mode = true, >> @@ -543,7 +547,16 @@ static int __init omap_rtc_probe(struct platform_device *pdev) >> if (rtc->type->has_32kclk_en) { >> reg = rtc_read(rtc, OMAP_RTC_OSC_REG); >> rtc_writel(rtc, OMAP_RTC_OSC_REG, >> - reg | OMAP_RTC_OSC_32KCLK_EN); >> + reg | OMAP_RTC_OSC_32KCLK_EN); >> + } >> + >> + /* Enable External clock as the source */ >> + >> + if (rtc->type->has_osc_ext_32k) { >> + rtc_writel(rtc, OMAP_RTC_OSC_REG, >> + (OMAP_RTC_OSC_EXT_32K | >> + rtc_read(rtc, OMAP_RTC_OSC_REG)) & >> + (~OMAP_RTC_OSC_OSC32K_GZ)); >> } > > How do we know that all systems have this external clock and that it > works OK? > AM335 and AM43X have the external clock feature which we choose using RTC_OSC_REG. I verified it works OK by seeing the RTC seconds ticking even after switching the source to the external 32k Clock. Regards, Keerthy -- 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
Hi, On 04/07/15 06:29, Keerthy wrote: > Hi Andrew, > > Apologies for replying late. > > On Wednesday 25 March 2015 04:29 AM, Andrew Morton wrote: >> On Tue, 3 Mar 2015 15:12:02 +0530 Keerthy <j-keerthy@ti.com> wrote: >> >>> Add external 32k clock feature. The internal clock will be gated during suspend. >>> Hence make use of the external 32k clock so that rtc is functional accross >>> suspend/resume. >>> >>> ... >>> >>> @@ -446,6 +449,7 @@ static const struct omap_rtc_device_type omap_rtc_default_type = { >>> >>> static const struct omap_rtc_device_type omap_rtc_am3352_type = { >>> .has_32kclk_en = true, >>> + .has_osc_ext_32k = true, >>> .has_kicker = true, >>> .has_irqwakeen = true, >>> .has_pmic_mode = true, >>> @@ -543,7 +547,16 @@ static int __init omap_rtc_probe(struct platform_device *pdev) >>> if (rtc->type->has_32kclk_en) { >>> reg = rtc_read(rtc, OMAP_RTC_OSC_REG); >>> rtc_writel(rtc, OMAP_RTC_OSC_REG, >>> - reg | OMAP_RTC_OSC_32KCLK_EN); >>> + reg | OMAP_RTC_OSC_32KCLK_EN); >>> + } >>> + >>> + /* Enable External clock as the source */ >>> + >>> + if (rtc->type->has_osc_ext_32k) { >>> + rtc_writel(rtc, OMAP_RTC_OSC_REG, >>> + (OMAP_RTC_OSC_EXT_32K | >>> + rtc_read(rtc, OMAP_RTC_OSC_REG)) & >>> + (~OMAP_RTC_OSC_OSC32K_GZ)); >>> } >> >> How do we know that all systems have this external clock and that it >> works OK? >> > > AM335 and AM43X have the external clock feature which we choose using > RTC_OSC_REG. I verified it works OK by seeing the RTC seconds ticking > even after switching the source to the external 32k Clock. AFAIU, this is related to the external (outside of SoC) oscillator, right? If so, there is a possibility to not assemble it on the board (at least on AM335) and use the internal clock source instead. There are dozens of boards out there that do not have the external oscillator assembled. Am I missing something?
Hi On Tuesday 07 April 2015 12:36 PM, Igor Grinberg wrote: > Hi, > > On 04/07/15 06:29, Keerthy wrote: >> Hi Andrew, >> >> Apologies for replying late. >> >> On Wednesday 25 March 2015 04:29 AM, Andrew Morton wrote: >>> On Tue, 3 Mar 2015 15:12:02 +0530 Keerthy <j-keerthy@ti.com> wrote: >>> >>>> Add external 32k clock feature. The internal clock will be gated during suspend. >>>> Hence make use of the external 32k clock so that rtc is functional accross >>>> suspend/resume. >>>> >>>> ... >>>> >>>> @@ -446,6 +449,7 @@ static const struct omap_rtc_device_type omap_rtc_default_type = { >>>> >>>> static const struct omap_rtc_device_type omap_rtc_am3352_type = { >>>> .has_32kclk_en = true, >>>> + .has_osc_ext_32k = true, >>>> .has_kicker = true, >>>> .has_irqwakeen = true, >>>> .has_pmic_mode = true, >>>> @@ -543,7 +547,16 @@ static int __init omap_rtc_probe(struct platform_device *pdev) >>>> if (rtc->type->has_32kclk_en) { >>>> reg = rtc_read(rtc, OMAP_RTC_OSC_REG); >>>> rtc_writel(rtc, OMAP_RTC_OSC_REG, >>>> - reg | OMAP_RTC_OSC_32KCLK_EN); >>>> + reg | OMAP_RTC_OSC_32KCLK_EN); >>>> + } >>>> + >>>> + /* Enable External clock as the source */ >>>> + >>>> + if (rtc->type->has_osc_ext_32k) { >>>> + rtc_writel(rtc, OMAP_RTC_OSC_REG, >>>> + (OMAP_RTC_OSC_EXT_32K | >>>> + rtc_read(rtc, OMAP_RTC_OSC_REG)) & >>>> + (~OMAP_RTC_OSC_OSC32K_GZ)); >>>> } >>> >>> How do we know that all systems have this external clock and that it >>> works OK? >>> >> >> AM335 and AM43X have the external clock feature which we choose using >> RTC_OSC_REG. I verified it works OK by seeing the RTC seconds ticking >> even after switching the source to the external 32k Clock. > > AFAIU, this is related to the external (outside of SoC) oscillator, right? > If so, there is a possibility to not assemble it on the board (at least > on AM335) and use the internal clock source instead. Yes. The register 'OMAP_RTC_OSC_REG' is part of the RTC IP register set so i assumed that it would be with the RTCs of those particular SoCs. > There are dozens of boards out there that do not have the external > oscillator assembled. > > Am I missing something? > Regards, Keerthy -- 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
On Fri, 10 Apr 2015 13:56:54 +0530 Keerthy <a0393675@ti.com> wrote: > >>> How do we know that all systems have this external clock and that it > >>> works OK? > >>> > >> > >> AM335 and AM43X have the external clock feature which we choose using > >> RTC_OSC_REG. I verified it works OK by seeing the RTC seconds ticking > >> even after switching the source to the external 32k Clock. > > > > AFAIU, this is related to the external (outside of SoC) oscillator, right? > > If so, there is a possibility to not assemble it on the board (at least > > on AM335) and use the internal clock source instead. > > Yes. The register 'OMAP_RTC_OSC_REG' is part of the RTC IP register set > so i assumed that it would be with the RTCs of those particular SoCs. I'm a bit lost here. AFAICT there's a risk that some manufacturers have not wired up the external oscillator, so this patch will break those systems. Do we know for sure that this cannot be the case? Is there any way in which we can get all systems working properly? If there's no way of auto-detecting an external oscillator then perhaps a module parameter is needed. If so, it would be better if the driver were to default to internal oscillator (ie: current behaviour), and the module parameter selects the external oscillator. This way, existing systems are unaffected by the kernel upgrade. -- 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
On 10/04/2015 at 13:37:30 -0700, Andrew Morton wrote : > Is there any way in which we can get all systems working properly? If > there's no way of auto-detecting an external oscillator then perhaps a > module parameter is needed. If so, it would be better if the driver > were to default to internal oscillator (ie: current behaviour), and the > module parameter selects the external oscillator. This way, existing > systems are unaffected by the kernel upgrade. > Actually, I would use the common clock framework, allowing to chose between the internal and an external clock source by using the "clocks" property. This would also allow to have the correct reference count on that 32k internal clock. It may not matter now but for example, not doing so became a problem on at91. As a fallback, if no clocks property is available, I would use the internal 32k to keep DT ABI backward compatibility.
diff --git a/drivers/rtc/rtc-omap.c b/drivers/rtc/rtc-omap.c index 8e5851a..4f803ca 100644 --- a/drivers/rtc/rtc-omap.c +++ b/drivers/rtc/rtc-omap.c @@ -107,6 +107,8 @@ /* OMAP_RTC_OSC_REG bit fields: */ #define OMAP_RTC_OSC_32KCLK_EN BIT(6) +#define OMAP_RTC_OSC_OSC32K_GZ BIT(4) +#define OMAP_RTC_OSC_EXT_32K BIT(3) /* OMAP_RTC_IRQWAKEEN bit fields: */ #define OMAP_RTC_IRQWAKEEN_ALARM_WAKEEN BIT(1) @@ -120,6 +122,7 @@ struct omap_rtc_device_type { bool has_32kclk_en; + bool has_osc_ext_32k; bool has_kicker; bool has_irqwakeen; bool has_pmic_mode; @@ -446,6 +449,7 @@ static const struct omap_rtc_device_type omap_rtc_default_type = { static const struct omap_rtc_device_type omap_rtc_am3352_type = { .has_32kclk_en = true, + .has_osc_ext_32k = true, .has_kicker = true, .has_irqwakeen = true, .has_pmic_mode = true, @@ -543,7 +547,16 @@ static int __init omap_rtc_probe(struct platform_device *pdev) if (rtc->type->has_32kclk_en) { reg = rtc_read(rtc, OMAP_RTC_OSC_REG); rtc_writel(rtc, OMAP_RTC_OSC_REG, - reg | OMAP_RTC_OSC_32KCLK_EN); + reg | OMAP_RTC_OSC_32KCLK_EN); + } + + /* Enable External clock as the source */ + + if (rtc->type->has_osc_ext_32k) { + rtc_writel(rtc, OMAP_RTC_OSC_REG, + (OMAP_RTC_OSC_EXT_32K | + rtc_read(rtc, OMAP_RTC_OSC_REG)) & + (~OMAP_RTC_OSC_OSC32K_GZ)); } /* clear old status */
Add external 32k clock feature. The internal clock will be gated during suspend. Hence make use of the external 32k clock so that rtc is functional accross suspend/resume. Signed-off-by: Keerthy <j-keerthy@ti.com> --- Tested on DRA7-EVM. drivers/rtc/rtc-omap.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)