linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: peter.chen@freescale.com (Peter Chen)
To: linux-arm-kernel@lists.infradead.org
Subject: [Patch v2 09/10] usb: chipidea: host: add ehci quirk for imx controller
Date: Thu, 24 Oct 2013 09:33:26 +0800	[thread overview]
Message-ID: <20131024013325.GC32138@shlinux1.ap.freescale.net> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1310231433390.1326-100000@iolanthe.rowland.org>

On Wed, Oct 23, 2013 at 02:45:39PM -0400, Alan Stern wrote:
> On Wed, 23 Oct 2013, Felipe Balbi wrote:
> 
> > Hi,
> > 
> > On Wed, Oct 23, 2013 at 10:46:09AM -0400, Alan Stern wrote:
> > > On Tue, 22 Oct 2013, Peter Chen wrote:
> > > 
> > > > When the port goes to suspend or finishes resme, it needs to
> > > > notify PHY, it is not a standard EHCI operation, so we add a
> > > > quirk for it.
> > > 
> > > Actually, this _should_ be a standard EHCI operation.  But we have to
> > > figure out a way to do it that will work on all platforms.
> > > 
> > > Felipe, any ideas?
> > 
> > isn't it enough to call usb_phy_set_suspend() at apropriate places ?
> > 
> > This should work on all platforms if the PHY driver is implemented
> > correctly.
> 
> On Tue, 22 Oct 2013, Peter Chen wrote:
> 
> > +				/*
> > +				 * notify the USB PHY, it is for global
> > +				 * suspend case.
> > +				 */
> > +				usb_phy_notify_suspend(hcd->phy,
> > +					USB_SPEED_HIGH);
> 
> Hmmm.  This calls usb_phy_notify_suspend(), and later on, 
> usb_phy_notify_resume().  AFAICT, those routines don't exist.
> 

Hi Alan and Felipe,

It is at my another patchset: "Add power management support for MXS PHY"
http://marc.info/?l=linux-usb&m=138242248913823&w=2
Since they are two things, one is from PHY, the other is for controller.
I split them to two patchset.

For these two notifications, please see:
[Patch v2 07/14] usb: phy: add notify suspend and resume callback
http://marc.info/?l=linux-usb&m=138242252713848&w=2

[Patch v2 08/14] usb: phy-mxs: Add implementation of
nofity_suspend and notify_resume
http://marc.info/?l=linux-usb&m=138242253313856&w=2

> Instead we have usb_phy_set_suspend(), as Felipe mentioned.  It has a
> different interface, because the second argument specifies whether we
> are entering or leaving suspend, not the connection speed.
> 
> Peter, you need to straighten this out.

.set_suspend is different with .notify_suspend.

.set_suspend is called when the PHY enters suspend.
.notify_suspend is called when port enters suspends (setting portsc.suspendM,
and stop sending SOF).

After calling .notify_suspend, the PHY may still need to access,
eg, set PHY wakeup setting, etc.

The .notify_suspend is in ehci operation, .set_suspend can be at
controller driver, since when to put the PHY enters suspend may
platform specific.

> 
> Also, there's the question of where are the appropriate places to make
> the calls?  After the root hub goes into suspend, I suppose.  But when
> is the right time during resume?
> 

If we can take it as standard EHCI operation, we need to put it at below
places:
	
For .notify_suspend, we need to put it after set portsc.suspendM.
- For selective suspend, at ehci_hub_control
- For global suspend, at ehci_bus_suspend

For .notify_resume, we need to put it after clear portsc.fpr,
it is similar for .notify_suspend
- For selective suspend, at ehci_hub_control
- For global suspend, at ehci_bus_resume

-- 

Best Regards,
Peter Chen

  reply	other threads:[~2013-10-24  1:33 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-22  6:23 [Patch v2 00/10] Add power management support for chipidea Peter Chen
2013-10-22  6:23 ` [Patch v2 01/10] usb: chipidea: Add power management support Peter Chen
2013-10-22  6:23 ` [Patch v2 02/10] usb: chipidea: imx: add " Peter Chen
2013-10-22  6:23 ` [Patch v2 03/10] usb: chipidea: add wakeup interrupt handler Peter Chen
2013-10-22  6:23 ` [Patch v2 04/10] usb: chipidea: usbmisc_imx: remove the controller's clock information Peter Chen
2013-10-22  6:23 ` [Patch v2 05/10] usb: chipidea: usbmisc_imx: add set_wakup API Peter Chen
2013-10-22  6:23 ` [Patch v2 06/10] usb: chipidea: imx: call set_wakeup when necessary Peter Chen
2013-10-22  6:23 ` [Patch v2 07/10] usb: chipidea: imx: Enable runtime pm support for imx6 SoC serial Peter Chen
2013-10-22  6:23 ` [Patch v2 08/10] usb: chipidea: host: add quirk for ehci operation Peter Chen
2013-10-23 14:42   ` Alan Stern
2013-10-24  0:47     ` Peter Chen
2013-10-24  5:51   ` Lothar Waßmann
2013-10-24  5:50     ` Peter Chen
2013-10-22  6:23 ` [Patch v2 09/10] usb: chipidea: host: add ehci quirk for imx controller Peter Chen
2013-10-23 14:46   ` Alan Stern
2013-10-23 18:06     ` Felipe Balbi
2013-10-23 18:45       ` Alan Stern
2013-10-24  1:33         ` Peter Chen [this message]
2013-10-24 12:04           ` Felipe Balbi
2013-10-24 12:11             ` Peter Chen
2013-10-22  6:23 ` [Patch v2 10/10] usb: chipidea: imx: Enable CI_HDRC_IMX_EHCI_QUIRK if the phy has notify APIs Peter Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131024013325.GC32138@shlinux1.ap.freescale.net \
    --to=peter.chen@freescale.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).