linux-kernel - RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 4 Jul 2011 11:50:47 -0400 (EDT)
From:	Alan Stern <stern@...land.harvard.edu>
To:	Partha Basak <p-basak2@...com>
cc:	Keshava Munegowda <keshava_mgowda@...com>,
	<linux-usb@...r.kernel.org>, <linux-omap@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, Felipe Balbi <balbi@...com>,
	Anand Gadiyar <gadiyar@...com>, <sameo@...ux.intel.com>,
	<parthab@...ia.ti.com>, <tony@...mide.com>,
	Kevin Hilman <khilman@...com>,
	Benoit Cousson <b-cousson@...com>, <paul@...an.com>,
	<johnstul@...ibm.com>, Vishwanath Sripathy <vishwanath.bs@...com>
Subject: RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support
 of ehci and ohci

On Mon, 4 Jul 2011, Partha Basak wrote:

> >I don't see any point in these routines (and likewise for
> >omap_ohci_suspend/resume).  When the whole system is going to sleep
> >anyway, what reason is there for enabling runtime PM on the parent
> >device?
> 
> Both for EHCI & OHCI, the clocks are owned by the parent (uhh-tll).
> 
> Calling pm_runtime_put_sync(dev->parent) within omap_ehci_suspend
> will turn-off the parent clocks in the Suspend path.
> 
> Similarly, calling pm_runtime_get_sync(dev->parent) within
> omap_ehci_resume
> will turn-on the parent clocks in the resume path.
> 
> This way, all reference counting are implicit within the Runtime PM layer
> and takes care of all combinations of only EHCI insmoded, OHCI insmoded,
> both insmoded etc.
> 
> When both EHCI & OHCI are suspended, parent clocks will actually be
> turned OFF and vice-versa.
> 
> Note that the parent per-se does not have any .suspend & .resume hooked
> up.

Why not?  That sounds like a big bug.

> At the end of the _probe of parent, the clocks are turned OFF.
> Subsequently, enabling
> the parent clocks are entirely done implicitly by the children get_sync()
> in their _probe.
> 
> Therefore while .suspend/.resume of children are called they call back
> into the parent to turn-off the clocks.

You have ignored a few very important points:

Firstly, system suspend is supposed to work even when runtime PM is not 
configured.

Secondly, the user can disable runtime PM via sysfs at any time.  This 
shouldn't mess up system suspend.

Basically, it's a bad idea to mix up system suspend with runtime PM.

Alan Stern

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/