Messages in this thread | | | Subject | Re: [RFC PATCH 2/2] rtc: s3c: Add s3c_rtc_{enable/disable}_clk in s3c_rtc_setfreq() | From | "pankaj.dubey" <> | Date | Tue, 05 Jul 2016 15:14:06 +0530 |
| |
Hi Alim,
On Tuesday 05 July 2016 02:22 PM, Alim Akhtar wrote: > > > On 07/05/2016 01:46 PM, Krzysztof Kozlowski wrote: >> On 07/04/2016 01:03 PM, Alim Akhtar wrote: >>> As per code flow it is possible that s3c_rtc_setfreq() might get called >>> with rtc clock disabled and in set_freq we perform h/w registers >>> read/write, >>> which might results in a kernel crash while probing rtc driver. >>> Below is one such case: >>> s3c_rtc_probe() >>> clk_prepare_enable(info->rtc_clk) // rtc clock enabled >>> s3c_rtc_gettime() // will enable clk if not done, and disable >>> it upon exit >>> s3c_rtc_setfreq() //then this will be called with clk disabled >> >> The indentation suggests levels of calls (chain) not sequence. This >> should be: >> s3c_rtc_probe() >> clk_prepare_enable(info->rtc_clk) // rtc clock enabled >> s3c_rtc_gettime() // will enable clk if not done, and disable it >> upon exit >> s3c_rtc_setfreq() //then this will be called with clk disabled >> >>> >>> This patch take cares of such issue by adding >>> s3c_rtc_{enable/disable}_clk in >>> s3c_rtc_setfreq(). >> >> What I don't get is that you wrote "it is *possible* that >> s3c_rtc_setfreq() *might* get called". From my understanding this will >> happen always because src_rtc_gettime() always disables the clocks. >> >> Why it does not happen always? >> > > Yes, you are right, it is always disabled when reaches s3c_rtc_setfreq(). > And I observed a kernel crash while testing on exynos7 platform in > s3c_rtc_setfreq() because clock was always disabled at this point. And > this patches fixes this issue. >
After addressing Krzysztof's review comments, Please feel free to add
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
For testing on Exynos7420 Tested-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Thanks, Pankaj Dubey
> >> Best regards, >> Krzysztof >> >
| |