Re: [PATCHv9 06/10] I2C: OMAP: Fix the crash in i2c remove

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Wolfram,

Wolfram Sang <w.sang@xxxxxxxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux