From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754499AbcGEIwe (ORCPT ); Tue, 5 Jul 2016 04:52:34 -0400 Received: from mailout3.samsung.com ([203.254.224.33]:49833 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754441AbcGEIwa (ORCPT ); Tue, 5 Jul 2016 04:52:30 -0400 X-AuditID: cbfee68e-f79266d000001428-5e-577b754c2c82 Message-id: <577B7537.4080506@samsung.com> Date: Tue, 05 Jul 2016 14:22:07 +0530 From: Alim Akhtar User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.5.0 MIME-version: 1.0 To: Krzysztof Kozlowski , rtc-linux@googlegroups.com, linux-kernel@vger.kernel.org Cc: alexandre.belloni@free-electrons.com, javier@osg.samsung.com, pankaj.dubey@samsung.com Subject: Re: [RFC PATCH 2/2] rtc: s3c: Add s3c_rtc_{enable/disable}_clk in s3c_rtc_setfreq() 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> In-reply-to: <577B6CD9.80605@samsung.com> Content-type: text/plain; charset=windows-1252; format=flowed Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprDIsWRmVeSWpSXmKPExsVy+t8zY12f0upwg8bDNhYd1xYzWbx5u4bJ 4vULQ4vLu+awWSza+oXdYn9nB6MDm8eTTRcZPfZMPMnmsaX/LrtH35ZVjB6fN8kFsEZx2aSk 5mSWpRbp2yVwZfxddZixYA5PRefLCWwNjI85uxg5OSQETCSOfbnMAmGLSVy4t56ti5GLQ0hg JaPE3M4DLDBFGxZ8ZgWxhQSWMkqcmmQMUfSAUeL+hBfMIAleAS2Jh8+aGEFsFgFViaVNR8Fs NgFtibvTtzB1MXJwiApESDy+IARRLijxY/I9FpCwiECuxNsPBiBhZoEYiUsb1oBNFBaIk1i7 7T0rxKpFjBLdp2eC3cMpoCmx5MJ2dogGW4kF79exQNjyEpvXvGUGaZAQOMUu0fJiByvEPQIS 3yYfAlsmISArsekAM8RfkhIHV9xgmcAoNgvJSbOQjJ2FZOwCRuZVjKKpBckFxUnpRUZ6xYm5 xaV56XrJ+bmbGCGR1reD8eYB60OMAhyMSjy8DmHV4UKsiWXFlbmHGE2BrpjILCWanA+M57yS eENjMyMLUxNTYyNzSzMlcd4EqZ/BQgLpiSWp2ampBalF8UWlOanFhxiZODilGhh7JuS/v9QY a55oonNwx41nxpOnCu2J/rX10Mo9ni/s9s7NX8gweePveZ9svHnP64lk5Ar7GJlu0V/5k28x 07LNS7Wy5wfaRl05un/dYu/M3wxd93y5n3o3rV2+ou/Eqm13xa2Wnhb9kffkvMqkn55b9+er BivFvvCqUBY38NiydPaR5VnWrVJVSizFGYmGWsxFxYkAHQggAq8CAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrJIsWRmVeSWpSXmKPExsVy+t9jAV2f0upwgxv7LCw6ri1msnjzdg2T xesXhhaXd81hs1i09Qu7xf7ODkYHNo8nmy4yeuyZeJLNY0v/XXaPvi2rGD0+b5ILYI1qYLTJ SE1MSS1SSM1Lzk/JzEu3VfIOjneONzUzMNQ1tLQwV1LIS8xNtVVy8QnQdcvMAbpASaEsMacU KBSQWFyspG+HaUJoiJuuBUxjhK5vSBBcj5EBGkhYx5jxd9VhxoI5PBWdLyewNTA+5uxi5OSQ EDCR2LDgMyuELSZx4d56NhBbSGApo8SpScZdjFxA9gNGifsTXjCDJHgFtCQePmtiBLFZBFQl ljYdBbPZBLQl7k7fwtTFyMEhKhAh8fiCEES5oMSPyfdYQMIiArkSbz8YgISZBWIkLm1YAzZR WCBOYu2296wQqxYxSnSfnskCkuAU0JRYcmE7O0SDrcSC9+tYIGx5ic1r3jJPYBSYhWTFLCRl s5CULWBkXsUokVqQXFCclJ5rmJdarlecmFtcmpeul5yfu4kRHM3PpHYwHtzlfohRgINRiYe3 YH5VuBBrYllxZe4hRgkOZiUR3sPF1eFCvCmJlVWpRfnxRaU5qcWHGE2BYTCRWUo0OR+YaPJK 4g2NTcyMLI3MLIxMzM2VxHkf/18XJiSQnliSmp2aWpBaBNPHxMEp1cB4Uv6Dh1VzkHbaxahz PCvqXz7ewtSmO8na/FTp3+zlXSn8a3/NFfvErtm7etvyQr6Fu6vqTixb17VSKOXZpCWBit1l H3XZsm/ZC/2v3Lnj0M5PSyJ0T2y54W/h6MF7/mf713nvV/FWP9pmGHjiwpw1/h3PAlJfKIll itVkvUo2qBEw5p+7v+6eEktxRqKhFnNRcSIAIu5kzfwCAAA= DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > Best regards, > Krzysztof >