linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman-l0cyMroinI0@public.gmane.org>
To: Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Cc: Shubhrajyoti D <shubhrajyoti-l0cyMroinI0@public.gmane.org>,
	linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org,
	tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org,
	Rajendra Nayak <rnayak-l0cyMroinI0@public.gmane.org>
Subject: Re: [PATCHv9 06/10] I2C: OMAP: Fix the crash in i2c remove
Date: Fri, 25 May 2012 14:51:34 -0700	[thread overview]
Message-ID: <878vggkjyx.fsf@ti.com> (raw)
In-Reply-To: <20120512181019.GA28973-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> (Wolfram Sang's message of "Sat, 12 May 2012 20:10:19 +0200")

Hi Wolfram,

Wolfram Sang <w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> writes:

> On Wed, May 02, 2012 at 08:02:11PM +0530, Shubhrajyoti D wrote:
>>     In omap_i2c_remove we are accessing the I2C_CON register without
>>     enabling the clocks. Fix the same by enabling the clocks and disabling
>>     it.

[...]

> I'd really like a comment from the PM experts if each and every driver
> has to ensure that the clocks are enabled on remove like this?

Yes, this is correct.

In fact, this is the goal of runtime PM.  The driver itself tells the PM
core (using runtime PM) when the device needs to be accessible and when
it doesn't.

Technically speaking, the it's up to the platform-specific runtime PM
implementation to decide whether or not the clocks are actually disable
or not  (e.g. due to wakeup latency requirements, it might decide not to
cut clocks.)

Because of that, the changelog should be reworded to say something like
"ensure device is accessible" instead of "enable the clocks", because
the runtime PM implementation does more than just manage clocks.

Kevin

P.S. It's great to see you helping out maintaining i2c drivers.  Thanks!

P.P.S. Before you merge this, I would strongly recommend we wait for a
       few more Tested-bys, and a bit more description about how this
       was tested.  We've been having quite a few problems with
       regressions introduced in OMAP drivers that have not been well
       reviewed or tested.

  parent reply	other threads:[~2012-05-25 21:51 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-02 14:32 [PATCHv9 00/10] I2C fixes Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 01/10] I2C: OMAP: make omap_i2c_unidle/idle functions depend on CONFIG_PM_RUNTIME Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 03/10] I2C: OMAP: Fix the interrupt clearing in OMAP4 Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 04/10] I2C: OMAP: Prevent the register access after pm_runtime_put in probe Shubhrajyoti D
2012-05-02 14:32 ` [PATCHv9 07/10] I2C: OMAP: Handle error check for pm runtime Shubhrajyoti D
     [not found]   ` <1335969135-20858-8-git-send-email-shubhrajyoti-l0cyMroinI0@public.gmane.org>
2012-05-25 22:06     ` Kevin Hilman
     [not found]       ` <87likfkjar.fsf-l0cyMroinI0@public.gmane.org>
2012-05-28 11:22         ` Shubhrajyoti
2012-05-02 14:32 ` [PATCHv9 08/10] I2C: OMAP: prevent the overwrite of the errata flags Shubhrajyoti D
     [not found] ` <1335969135-20858-1-git-send-email-shubhrajyoti-l0cyMroinI0@public.gmane.org>
2012-05-02 14:32   ` [PATCHv9 02/10] I2C: OMAP: Fix the mismatch of pm_runtime enable and disable Shubhrajyoti D
2012-05-25 21:57     ` Kevin Hilman
     [not found]       ` <87wr3zkjp7.fsf-l0cyMroinI0@public.gmane.org>
2012-05-28  9:59         ` Shubhrajyoti
2012-05-02 14:32   ` [PATCHv9 05/10] I2C: OMAP: Don't check if wait_for_completion_timeout() returns less than zero Shubhrajyoti D
2012-05-02 14:32   ` [PATCHv9 06/10] I2C: OMAP: Fix the crash in i2c remove Shubhrajyoti D
     [not found]     ` <1335969135-20858-7-git-send-email-shubhrajyoti-l0cyMroinI0@public.gmane.org>
2012-05-12 18:10       ` Wolfram Sang
2012-05-14 11:26         ` Shubhrajyoti
     [not found]         ` <20120512181019.GA28973-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
2012-05-25 21:51           ` Kevin Hilman [this message]
2012-05-02 14:32   ` [PATCHv9 09/10] I2C: OMAP: Do not set the XUDF(Transmit underflow) if the underflow is not reached Shubhrajyoti D
2012-05-25 22:09     ` Kevin Hilman
2012-05-02 14:32   ` [PATCHv9 10/10] I2C: OMAP: Rename the 1p153 to the erratum id i462 Shubhrajyoti D
2012-05-25 22:13   ` [PATCHv9 00/10] I2C fixes Kevin Hilman
2012-05-28  9:52     ` Shubhrajyoti
2012-05-12 18:10 ` Wolfram Sang
2012-05-25 23:40 ` Kevin Hilman
     [not found]   ` <87obpbhls0.fsf-l0cyMroinI0@public.gmane.org>
2012-05-28  9:54     ` Shubhrajyoti

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=878vggkjyx.fsf@ti.com \
    --to=khilman-l0cymroini0@public.gmane.org \
    --cc=ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=rnayak-l0cyMroinI0@public.gmane.org \
    --cc=shubhrajyoti-l0cyMroinI0@public.gmane.org \
    --cc=tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org \
    --cc=w.sang-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).