On 9/20/2010 7:30 PM, Kevin Hilman wrote:
Ohad Ben-Cohen<[email protected]> writes:
Hi Hari,
On Thu, Aug 12, 2010 at 12:44 AM, Kanigeri, Hari<[email protected]> wrote:
+/* Attempt to acquire a spinlock once */
+int hwspinlock_trylock(struct hwspinlock *handle)
+{
+ int retval = 0;
+
+ if (WARN_ON(handle == NULL))
+ return -EINVAL;
+
+ if (WARN_ON(in_irq()))
+ return -EPERM;
+
+ if (pm_runtime_get(&handle->pdev->dev)< 0)
+ return -ENODEV;
+
+ /* Attempt to acquire the lock by reading from it */
+ retval = readl(handle->lock_reg);
+
+ if (retval == HWSPINLOCK_BUSY)
+ pm_runtime_put(&handle->pdev->dev);
Any reason for using pm_runtime_put instead of pm_runtime_put_sync?
Using pm_runtime_gett_sync& pm_runtime_put_sync have been recommended by
Kevin Hilman.
Actually is there a need to call pm_runtime_put_sync for hwspinlock ? Spinlocks
are used by the co-processors and we have to ensure that the device doesn't
enter some low power mode without the knowledge of Co-processor. I don't think
run time PM is needed for hwspinlock.
Just doing pm_runtime_get_sync at probe time for all the spinlock instances
should be good.
It would probably make more sense to call pm_runtime_get_sync during
hwspinlock_request{_specific}, and then call pm_runtime_put during
hwspinlock_free.
This way the runtime PM's usage_count will reflect the number of locks
that are actually used, and if that number drops to (or never go
beyond) zero, it is desirable to have the hwspinlock's clock disabled.
This is also safe since no other core will use the hwspinlock if it
wasn't requested by the MPU beforehand (and if it does, we better know
about it and fix it).
FWIW, I agree with Ohad.
An additional benefit of using runtime PM is that the runtime PM core is
growing some useful debug and statistics features so that userspace
tools (including newer versions of powertop) can present useful stats
about which devices are active and how often etc.
And I agree with both of you.
Just to explain the context to Hari. IP like hwspinlock or mailbox does
not require functional clock to work. Because of that you can use it
without explicit clock enable thanks to the PRCM automatic modes.
So in theory you do not have to enable / idle hwmod for each spinlock
request or even during the probe, and this is what the original patches
from Simon were doing. I asked Simon to add explicit pm_runtime call,
even if useless, because of the reason mentioned by Kevin, but also
because the driver should not assume any automatic mode in the HW. That
IP can be used in other SoC that will not have PRCM at all.
In our case these calls will be mostly NOP, but that's still needed.
Regards,
Benoit
Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
