Re: [PATCH 3/8] rtc: omap: Add external clock enabling support

2015-08-07 Thread Keerthy



On Thursday 06 August 2015 03:21 PM, Tony Lindgren wrote:

* Alexandre Belloni  [150806 02:50]:

On 06/08/2015 at 12:36:54 +0300, Grygorii Strashko wrote :

Pls, correct me if I'm not right. Is below what you propose?

Doard dts:
/ {
  rtc_32k_ext_clk: rtc_osc_xi_clkin32_ext {
#clock-cells = <0>;
compatible = "fixed-clock";
clock-frequency = <32000>;
clock-output-names = "rtc_osc_xi_clkin32";
   };
}

  &rtc {
status = "okay";
clocks = <&sys_32k_ck>, <&rtc_32k_ext_clk>;
[optional] clock-names = "int-clk", "ext-clk";
  };

Driver:
1) clk0 is mandatory, internal clock source
2) clk1 is optional, external clock source, so
if present - RTC driver can switch to use ext clock source


Thanks Grygorii. I will implement it this way.





Absolutely!


Sounds good to me too.

Tony
--
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


--
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/8] rtc: omap: Add external clock enabling support

2015-08-06 Thread Tony Lindgren
* Alexandre Belloni  [150806 02:50]:
> On 06/08/2015 at 12:36:54 +0300, Grygorii Strashko wrote :
> > Pls, correct me if I'm not right. Is below what you propose?
> > 
> > Doard dts:
> > / {
> >  rtc_32k_ext_clk: rtc_osc_xi_clkin32_ext {
> > #clock-cells = <0>;
> > compatible = "fixed-clock";
> > clock-frequency = <32000>;
> > clock-output-names = "rtc_osc_xi_clkin32";
> >   };
> > }
> > 
> >  &rtc {
> > status = "okay";
> > clocks = <&sys_32k_ck>, <&rtc_32k_ext_clk>;
> > [optional] clock-names = "int-clk", "ext-clk";
> >  };
> > 
> > Driver:
> > 1) clk0 is mandatory, internal clock source
> > 2) clk1 is optional, external clock source, so
> > if present - RTC driver can switch to use ext clock source
> > 
> 
> Absolutely!

Sounds good to me too.

Tony
--
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/8] rtc: omap: Add external clock enabling support

2015-08-06 Thread Alexandre Belloni
On 06/08/2015 at 12:36:54 +0300, Grygorii Strashko wrote :
> Pls, correct me if I'm not right. Is below what you propose?
> 
> Doard dts:
> / {
>  rtc_32k_ext_clk: rtc_osc_xi_clkin32_ext {
>   #clock-cells = <0>;
>   compatible = "fixed-clock";
>   clock-frequency = <32000>;
>   clock-output-names = "rtc_osc_xi_clkin32";
>   };
> }
> 
>  &rtc {
>   status = "okay";
>   clocks = <&sys_32k_ck>, <&rtc_32k_ext_clk>;
>   [optional] clock-names = "int-clk", "ext-clk";
>  };
> 
> Driver:
> 1) clk0 is mandatory, internal clock source
> 2) clk1 is optional, external clock source, so
> if present - RTC driver can switch to use ext clock source
> 

Absolutely!

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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/8] rtc: omap: Add external clock enabling support

2015-08-06 Thread Alexandre Belloni
On 06/08/2015 at 07:39:52 +0530, Keerthy wrote :
> On Wednesday 05 August 2015 06:05 PM, Alexandre Belloni wrote:
> >On 05/08/2015 at 17:31:22 +0530, Keerthy wrote :
> >>This is a special one where in the enable bit is present in the rtc register
> >>space and not in the prcm register space. Since there was a concern on the
> >>external clock not being present i added a board dts flag.
> >>
> >
> >So you mean this external clock is coming internally from the SoC?
> 
> No what i meant is external clock is coming from Oscillator OSC1 @32.768KHz
> but the controlling bits are part of rtc register space.
> 
> TRM: http://www.ti.com/lit/ug/spruhl7c/spruhl7c.pdf
> 
> Section: 19.4.3.2 Clock Source Page 2836
> 
> Also register details:
> 19.4.5.19 RTCSS_OSC_REG Register (offset = 54h) [reset = 10h]
> 
> Page 2865.
> 

This confirms what I'm saying. Your issue here is that the driver is not
properly taking the clocks so when the PRCM is disabling CLK_32KHZ, you
end up without any clock.

You can use the clocks property in the device tree and pass two clocks,
the prcm one and the external crystal/external oscillator.
In the driver, you get both clock, clk_get_rate on the external one will
help you know whether it is populated or not (this will be 0 or 32768).
It is is populated, use it by writing 32KCLK_SEL.

Bonus points if you use the clock-accuracy and decide to switch between
PRCM and the external clock when going to suspend and resuming. I guess
an external RC oscillator is quite bad versus the PRCM.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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/8] rtc: omap: Add external clock enabling support

2015-08-06 Thread Grygorii Strashko
Hi Alexandre,
On 08/05/2015 02:43 PM, Alexandre Belloni wrote:
> On 05/08/2015 at 13:41:19 +0200, Alexandre Belloni wrote :
>> Hi,
>>
>> On 05/08/2015 at 04:13:17 -0700, Tony Lindgren wrote :
>>> * Keerthy  [150805 03:53]:
 Based on the board property switch the source from internal
 to external clock. Switching to external source is needed for
 rtcwake to work in low power modes.
>>>
>>> I think this is better handled based on the compatible string
>>> in the device driver rather than introducing a custom dts
>>> property for it. You can just set the quirk flag in the driver
>>> probe based on the compatible.
>>>
>>
>> Why not use the clocks property? Then you can pass an external clock. If
>> it is present you can even get its rate if this is needed at some point
>> in the future. You could also disable it when going to suspend.
>>
> 
> Actually, that was already my suggestion back in april:
> http://patchwork.ozlabs.org/patch/445631/
> 
> (Please Cc: the rtc mailing list for RTC related patches so that they
> get picked up by patchwork).
> 

Pls, correct me if I'm not right. Is below what you propose?

Doard dts:
/ {
 rtc_32k_ext_clk: rtc_osc_xi_clkin32_ext {
#clock-cells = <0>;
compatible = "fixed-clock";
clock-frequency = <32000>;
clock-output-names = "rtc_osc_xi_clkin32";
  };
}

 &rtc {
status = "okay";
clocks = <&sys_32k_ck>, <&rtc_32k_ext_clk>;
[optional] clock-names = "int-clk", "ext-clk";
 };

Driver:
1) clk0 is mandatory, internal clock source
2) clk1 is optional, external clock source, so
if present - RTC driver can switch to use ext clock source

-- 
regards,
-grygorii
--
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/8] rtc: omap: Add external clock enabling support

2015-08-05 Thread Keerthy



On Wednesday 05 August 2015 06:05 PM, Alexandre Belloni wrote:

On 05/08/2015 at 17:31:22 +0530, Keerthy wrote :

This is a special one where in the enable bit is present in the rtc register
space and not in the prcm register space. Since there was a concern on the
external clock not being present i added a board dts flag.



So you mean this external clock is coming internally from the SoC?


No what i meant is external clock is coming from Oscillator OSC1 
@32.768KHz but the controlling bits are part of rtc register space.


TRM: http://www.ti.com/lit/ug/spruhl7c/spruhl7c.pdf

Section: 19.4.3.2 Clock Source Page 2836

Also register details:
19.4.5.19 RTCSS_OSC_REG Register (offset = 54h) [reset = 10h]

Page 2865.

Regards,
Keerthy



Do you have a link to the datasheet?


--
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/8] rtc: omap: Add external clock enabling support

2015-08-05 Thread Alexandre Belloni
On 05/08/2015 at 17:31:22 +0530, Keerthy wrote :
> This is a special one where in the enable bit is present in the rtc register
> space and not in the prcm register space. Since there was a concern on the
> external clock not being present i added a board dts flag.
> 

So you mean this external clock is coming internally from the SoC?

Do you have a link to the datasheet?

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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/8] rtc: omap: Add external clock enabling support

2015-08-05 Thread Keerthy



On Wednesday 05 August 2015 05:13 PM, Alexandre Belloni wrote:

On 05/08/2015 at 13:41:19 +0200, Alexandre Belloni wrote :

Hi,

On 05/08/2015 at 04:13:17 -0700, Tony Lindgren wrote :

* Keerthy  [150805 03:53]:

Based on the board property switch the source from internal
to external clock. Switching to external source is needed for
rtcwake to work in low power modes.


I think this is better handled based on the compatible string
in the device driver rather than introducing a custom dts
property for it. You can just set the quirk flag in the driver
probe based on the compatible.



Why not use the clocks property? Then you can pass an external clock. If
it is present you can even get its rate if this is needed at some point
in the future. You could also disable it when going to suspend.



Actually, that was already my suggestion back in april:
http://patchwork.ozlabs.org/patch/445631/

(Please Cc: the rtc mailing list for RTC related patches so that they
get picked up by patchwork).


Hi Alexandre,

This is a special one where in the enable bit is present in the rtc 
register space and not in the prcm register space. Since there was a 
concern on the external clock not being present i added a board dts flag.


Regards,
Keerthy



--
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/8] rtc: omap: Add external clock enabling support

2015-08-05 Thread Alexandre Belloni
On 05/08/2015 at 13:41:19 +0200, Alexandre Belloni wrote :
> Hi,
> 
> On 05/08/2015 at 04:13:17 -0700, Tony Lindgren wrote :
> > * Keerthy  [150805 03:53]:
> > > Based on the board property switch the source from internal
> > > to external clock. Switching to external source is needed for
> > > rtcwake to work in low power modes.
> > 
> > I think this is better handled based on the compatible string
> > in the device driver rather than introducing a custom dts
> > property for it. You can just set the quirk flag in the driver
> > probe based on the compatible.
> > 
> 
> Why not use the clocks property? Then you can pass an external clock. If
> it is present you can even get its rate if this is needed at some point
> in the future. You could also disable it when going to suspend.
> 

Actually, that was already my suggestion back in april:
http://patchwork.ozlabs.org/patch/445631/

(Please Cc: the rtc mailing list for RTC related patches so that they
get picked up by patchwork).

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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/8] rtc: omap: Add external clock enabling support

2015-08-05 Thread Alexandre Belloni
Hi,

On 05/08/2015 at 04:13:17 -0700, Tony Lindgren wrote :
> * Keerthy  [150805 03:53]:
> > Based on the board property switch the source from internal
> > to external clock. Switching to external source is needed for
> > rtcwake to work in low power modes.
> 
> I think this is better handled based on the compatible string
> in the device driver rather than introducing a custom dts
> property for it. You can just set the quirk flag in the driver
> probe based on the compatible.
> 

Why not use the clocks property? Then you can pass an external clock. If
it is present you can even get its rate if this is needed at some point
in the future. You could also disable it when going to suspend.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
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/8] rtc: omap: Add external clock enabling support

2015-08-05 Thread Tony Lindgren
* Keerthy  [150805 03:53]:
> Based on the board property switch the source from internal
> to external clock. Switching to external source is needed for
> rtcwake to work in low power modes.

I think this is better handled based on the compatible string
in the device driver rather than introducing a custom dts
property for it. You can just set the quirk flag in the driver
probe based on the compatible.

Regards,

Tony
--
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