From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757943Ab1GDPut (ORCPT ); Mon, 4 Jul 2011 11:50:49 -0400 Received: from netrider.rowland.org ([192.131.102.5]:39579 "HELO netrider.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1757197Ab1GDPur (ORCPT ); Mon, 4 Jul 2011 11:50:47 -0400 Date: Mon, 4 Jul 2011 11:50:47 -0400 (EDT) From: Alan Stern X-X-Sender: stern@netrider.rowland.org To: Partha Basak cc: Keshava Munegowda , , , , Felipe Balbi , Anand Gadiyar , , , , Kevin Hilman , Benoit Cousson , , , Vishwanath Sripathy Subject: RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci In-Reply-To: <11640ef76f97628579e457b5c6a74cc3@mail.gmail.com> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alan Stern Subject: RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci Date: Mon, 4 Jul 2011 11:50:47 -0400 (EDT) Message-ID: References: <11640ef76f97628579e457b5c6a74cc3@mail.gmail.com> Mime-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Return-path: Received: from netrider.rowland.org ([192.131.102.5]:37477 "HELO netrider.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1757235Ab1GDPur (ORCPT ); Mon, 4 Jul 2011 11:50:47 -0400 In-Reply-To: <11640ef76f97628579e457b5c6a74cc3@mail.gmail.com> Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: Partha Basak Cc: Keshava Munegowda , linux-usb@vger.kernel.org, linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org, Felipe Balbi , Anand Gadiyar , sameo@linux.intel.com, parthab@india.ti.com, tony@atomide.com, Kevin Hilman , Benoit Cousson , paul@pwsan.com, johnstul@us.ibm.com, Vishwanath Sripathy 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