Re: [PATCH 8/9 v3] usb : musb: Using runtime pm apis for musb.

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

 



Hi,

On Thu, Sep 23, 2010 at 02:51:12AM -0500, Kalliguddi, Hema wrote:
these two should a separate series, otherwise it's difficult for
different maintainers to decide what they need to pick up :-)

I don't mind

if you're re-sending already, please do split :-)

I would rather remove these, adding ifdefs is bad :-( Unless
other archs
(blackfin, davinci) would have problems if we remove those.

If we remove this then how the clock will be enabled for the other
platforms where runtime pm is not used?

yeah, let's see what other archs will say...

@@ -2457,9 +2465,26 @@ static int musb_resume_noirq(struct devi
	return 0;
}

+static int musb_runtime_suspend(struct device *dev)
+{
+	struct musb	*musb = dev_to_musb(dev);

missing lock ??
I am wondering whether we need the lock here? As these functions are supposed to be
Called by runtime pm framework only.

is that done with IRQs disabled ?

+	musb_save_context(musb);

shouldn't you disable the clock here ?

This hook is called when we call pm_runtime_put_sync api which takes
care of disabling clocks and configuring the sysconfig register.

but those are methods you have provided, right ? I mean, in the end
it'll call musb_runtime_suspend() and musb_runtime_resume() ?? Or am I
missing something ?

should you pm_runtime_put_sync(dev) here ??

Calling pm_runtime_put_sync leading to crash as
driver_detach calls __device_release_driver which intern calls
pm_runtime_put_sync function, with this the musb clocks are already disabled
And usecount is 0.

looks weird though, I'll have to read that code more carefully when you
send another version.

-	l = musb_readl(musb->mregs, OTG_SYSCONFIG);
-	l &= ~ENABLEWAKEUP;	/* disable wakeup */
-	musb_writel(musb->mregs, OTG_SYSCONFIG, l);
+	pm_runtime_enable(dev);
+	pm_runtime_get_sync(dev);

seems like you're going to call pm_runtime_get_sync twice ? once here
and another time on musb_resume_noirq(). why ?

pm_runtime_get_sync is called in the musb_platform_resume function
during the initialization of the driver. Where we need to enable the clocks
and put the sysconfig registers to known configurations.

pm_runtime_get_sync is called in musb_resume_noirq() to re-enable the cloks
and configure the sysconfig because the clocks are disabled in musb_suspend()
by calling pm_runtime_put_sync() during global suspend/resume.

hmm, also weird logic. I'll wait for next version and read that code
with more care.

--
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux