From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754369AbcGEI5J (ORCPT ); Tue, 5 Jul 2016 04:57:09 -0400 Received: from mailout4.w1.samsung.com ([210.118.77.14]:22279 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751003AbcGEI5E (ORCPT ); Tue, 5 Jul 2016 04:57:04 -0400 X-AuditID: cbfec7f5-f792a6d000001302-dc-577b7630ff1b Subject: Re: [RFC PATCH 2/2] rtc: s3c: Add s3c_rtc_{enable/disable}_clk in s3c_rtc_setfreq() To: Alim Akhtar , rtc-linux@googlegroups.com, linux-kernel@vger.kernel.org References: <1467630195-6929-1-git-send-email-alim.akhtar@samsung.com> <1467630195-6929-2-git-send-email-alim.akhtar@samsung.com> <577B6CD9.80605@samsung.com> <577B7537.4080506@samsung.com> Cc: alexandre.belloni@free-electrons.com, javier@osg.samsung.com, pankaj.dubey@samsung.com From: Krzysztof Kozlowski Message-id: <577B762F.6020609@samsung.com> Date: Tue, 05 Jul 2016 10:56:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-version: 1.0 In-reply-to: <577B7537.4080506@samsung.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrGLMWRmVeSWpSXmKPExsVy+t/xy7oGZdXhBs03bCw6ri1msngwbxub xZu3a5gsXr8wtLi8aw6bxaKtX9gt9nd2MDqwezzZdJHRY8/Ek2weW/rvsnv0bVnF6PF5k1wA axSXTUpqTmZZapG+XQJXxuJ/axgLVvNXLPpxir2BcT1PFyMnh4SAicSTl00sELaYxIV769m6 GLk4hASWMkqs+fqUGcJ5xiix/M4xJpAqYYE4ibXb3rN2MXJwiAikSrQ8NYSoOcgoMbvrKCNI DbNAjMSlDWuYQWw2AWOJzcuXsIHYvAJaEjcX3QOaw87BIqAqsRwsKioQITFr+w8miApBiR+T 77GATOcU0JZYOyccxGQW0JO4f1ELYra8xOY1b5knMArMQtIwC6FqFpKqBYzMqxhFU0uTC4qT 0nON9IoTc4tL89L1kvNzNzFCQvvrDsalx6wOMQpwMCrx8BbMrwoXYk0sK67MPcQowcGsJMJ7 uLg6XIg3JbGyKrUoP76oNCe1+BCjNAeLkjjvzF3vQ4QE0hNLUrNTUwtSi2CyTBycUg2MJ3jd +IN5zpv43LPSjTg5yekth8unR2flHJsrjCzq1e7OrJu3yyhgW179XlP2f9+V2spbe5UXzRT4 EjVv8qOpp3UfsbkHm/Ttyl+oUhx2hOHQl2PpR/mXuh2f3hbgHX5zh/Y/sZ0nFD5YiXZdO8Uo 0aI+7WWq9A6pA1Xys3vC263sheX3S1kosRRnJBpqMRcVJwIA4Ir4dWkCAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/05/2016 10:52 AM, 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. Ok, thanks! Could you adjust the commit message mentioning that it always causes crash on Exynos 7 platform and adding: Fixes: 24e1455493da ("drivers/rtc/rtc-s3c.c: delete duplicate clock control") Cc: Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof