All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Alim Akhtar <alim.akhtar@samsung.com>,
	rtc-linux@googlegroups.com, linux-kernel@vger.kernel.org
Cc: alexandre.belloni@free-electrons.com, javier@osg.samsung.com,
	pankaj.dubey@samsung.com
Subject: [rtc-linux] Re: [RFC PATCH 2/2] rtc: s3c: Add s3c_rtc_{enable/disable}_clk in s3c_rtc_setfreq()
Date: Tue, 05 Jul 2016 10:16:25 +0200	[thread overview]
Message-ID: <577B6CD9.80605@samsung.com> (raw)
In-Reply-To: <1467630195-6929-2-git-send-email-alim.akhtar@samsung.com>

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?

Best regards,
Krzysztof

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

WARNING: multiple messages have this Message-ID (diff)
From: Krzysztof Kozlowski <k.kozlowski@samsung.com>
To: Alim Akhtar <alim.akhtar@samsung.com>,
	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()
Date: Tue, 05 Jul 2016 10:16:25 +0200	[thread overview]
Message-ID: <577B6CD9.80605@samsung.com> (raw)
In-Reply-To: <1467630195-6929-2-git-send-email-alim.akhtar@samsung.com>

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?

Best regards,
Krzysztof

  reply	other threads:[~2016-07-05  8:16 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-04 11:03 [rtc-linux] [RFC PATCH 1/2] rtc: s3c: Remove unnecessary call to disable already disabled clock Alim Akhtar
2016-07-04 11:03 ` Alim Akhtar
2016-07-04 11:03 ` [rtc-linux] [RFC PATCH 2/2] rtc: s3c: Add s3c_rtc_{enable/disable}_clk in s3c_rtc_setfreq() Alim Akhtar
2016-07-04 11:03   ` Alim Akhtar
2016-07-05  8:16   ` Krzysztof Kozlowski [this message]
2016-07-05  8:16     ` Krzysztof Kozlowski
2016-07-05  8:52     ` [rtc-linux] " Alim Akhtar
2016-07-05  8:52       ` Alim Akhtar
2016-07-05  8:56       ` [rtc-linux] " Krzysztof Kozlowski
2016-07-05  8:56         ` Krzysztof Kozlowski
2016-07-05  9:43         ` [rtc-linux] " pankaj.dubey
2016-07-05  9:43           ` pankaj.dubey
2016-07-05  9:44       ` [rtc-linux] " pankaj.dubey
2016-07-05  9:44         ` pankaj.dubey
2016-07-05  7:18 ` [rtc-linux] Re: [RFC PATCH 1/2] rtc: s3c: Remove unnecessary call to disable already disabled clock Krzysztof Kozlowski
2016-07-05  7:18   ` Krzysztof Kozlowski
2016-07-05  8:46   ` [rtc-linux] " Alim Akhtar
2016-07-05  8:46     ` Alim Akhtar
2016-07-05  8:50     ` [rtc-linux] " Krzysztof Kozlowski
2016-07-05  8:50       ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=577B6CD9.80605@samsung.com \
    --to=k.kozlowski@samsung.com \
    --cc=alexandre.belloni@free-electrons.com \
    --cc=alim.akhtar@samsung.com \
    --cc=javier@osg.samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pankaj.dubey@samsung.com \
    --cc=rtc-linux@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.