From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754630AbcGEJlx (ORCPT ); Tue, 5 Jul 2016 05:41:53 -0400 Received: from mailout4.samsung.com ([203.254.224.34]:57196 "EHLO mailout4.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754569AbcGEJlu (ORCPT ); Tue, 5 Jul 2016 05:41:50 -0400 X-AuditID: cbfee68f-f79476d000001429-59-577b80d7afec Subject: Re: [RFC PATCH 2/2] rtc: s3c: Add s3c_rtc_{enable/disable}_clk in s3c_rtc_setfreq() To: Alim Akhtar , Krzysztof Kozlowski , 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 From: "pankaj.dubey" Message-id: <577B8166.8060208@samsung.com> Date: Tue, 05 Jul 2016 15:14:06 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.2 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+NgFprNIsWRmVeSWpSXmKPExsVy+t8zfd3rDdXhBt97+C06ri1msngwbxub xZu3a5gsXr8wtLi8aw6bxf7ODkYHNo8nmy4yeuyZeJLNY0v/XXaPvi2rGD0+b5ILYI3isklJ zcksSy3St0vgymheq1Qwjb9ix6mvbA2MvTxdjJwcEgImEitvXWaCsMUkLtxbz9bFyMUhJLCS UeL7mVOMMEVP9jaygNhCAksZJZ7t8YQo+s4o8WLTdmaQhLBAnMTabe9ZQRIiAtMYJT4vnsIE UXWQUWJ211GwUcwCDhKr500D62AT0JeY/ngbG4jNK6Al8fjvVrA4i4CqxO/l/8HWiQpESKxe d40ZokZQ4sfke0BxDg5OAW2JtXPCQUxmAT2J+xe1IKbLS2xe85YZZK2EwDF2ic0LNrNCjBSQ +Db5EFirhICsxKYDzBCPSUocXHGDZQKj2CwkC2YhTJ2FZOoCRuZVjKKpBckFxUnpRcZ6xYm5 xaV56XrJ+bmbGCFx1r+D8e4B60OMAhyMSjy8FcuqwoVYE8uKK3MPMZoCHTGRWUo0OR8YzXkl 8YbGZkYWpiamxkbmlmZK4rwLpX4GCwmkJ5akZqemFqQWxReV5qQWH2Jk4uCUamBMrKucZvtq 0rRy0WcW+7kf+eb46Hm8/u8w2aW6LLnBXoNtVob9g4t3rG+nT7Ayny+waksvk8iXrX5pN8yS Vy5e8TLx0vo1c/S8p2a4Xy479oDDKM9xTQvP6UVKqwXiVcRvd3QwuSx4u7X/pNOJm9Mdlfiv HVjMXrx8pjxDxoxbV2Z7xOwT4bFUYinOSDTUYi4qTgQAtyOAjK4CAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrBIsWRmVeSWpSXmKPExsVy+t9jAd3rDdXhBi0PuSw6ri1msngwbxub xZu3a5gsXr8wtLi8aw6bxf7ODkYHNo8nmy4yeuyZeJLNY0v/XXaPvi2rGD0+b5ILYI1qYLTJ SE1MSS1SSM1Lzk/JzEu3VfIOjneONzUzMNQ1tLQwV1LIS8xNtVVy8QnQdcvMAbpASaEsMacU KBSQWFyspG+HaUJoiJuuBUxjhK5vSBBcj5EBGkhYx5jRvFapYBp/xY5TX9kaGHt5uhg5OSQE TCSe7G1kgbDFJC7cW88GYgsJLGWUeLbHs4uRC8j+zijxYtN2ZpCEsECcxNpt71lBEiIC0xgl Pi+ewgRRdZBRYnbXUUaQKmYBB4nV86aBdbAJ6EtMf7wNbCyvgJbE479bweIsAqoSv5f/B1st KhAhsXrdNWaIGkGJH5PvAcU5ODgFtCXWzgkHMZkF9CTuX9SCmC4vsXnNW+YJjAKzkDTMQqia haRqASPzKkaJ1ILkguKk9FzDvNRyveLE3OLSvHS95PzcTYzgWH4mtYPx4C73Q4wCHIxKPLwF 86vChVgTy4orcw8xSnAwK4nw6tVWhwvxpiRWVqUW5ccXleakFh9iNAX6YiKzlGhyPjDN5JXE GxqbmBlZGplZGJmYmyuJ8z7+vy5MSCA9sSQ1OzW1ILUIpo+Jg1OqgZFPpbhzWkqTsE2kNdOH m+YX39ZqGX666xEWZpbAtvbmpKMvzDy2NHYLMtQ8clnqv6/IYErnv2N6smtuTjs8p1Fk9ck/ zPW8s40/BFX+jRcszVP6HBw5fcfe7bWfothd9vKtn+Eo6LRkI9dBnfb7HOlOMlzM/Vcl3fYf UWG3569YbJ9/gs2/XYmlOCPRUIu5qDgRALzsL2j7AgAA 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 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 For testing on Exynos7420 Tested-by: Pankaj Dubey Thanks, Pankaj Dubey > >> Best regards, >> Krzysztof >> >