On Fri, Dec 13, 2013 at 12:32 PM, Felipe Balbi <balbi@xxxxxx> wrote: > On Fri, Dec 13, 2013 at 09:23:38AM +0800, Peter Chen wrote: >> Implementation of notify_suspend and notify_resume will be different >> according to mxs_phy_data->flags. >> >> Signed-off-by: Peter Chen <peter.chen@xxxxxxxxxxxxx> >> --- >> drivers/usb/phy/phy-mxs-usb.c | 55 ++++++++++++++++++++++++++++++++++++++--- >> 1 files changed, 51 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c >> index 0ef930a..e3df53f 100644 >> --- a/drivers/usb/phy/phy-mxs-usb.c >> +++ b/drivers/usb/phy/phy-mxs-usb.c >> @@ -166,8 +166,8 @@ static int mxs_phy_suspend(struct usb_phy *x, int suspend) >> static int mxs_phy_on_connect(struct usb_phy *phy, >> enum usb_device_speed speed) >> { >> - dev_dbg(phy->dev, "%s speed device has connected\n", >> - (speed == USB_SPEED_HIGH) ? "high" : "non-high"); >> + dev_dbg(phy->dev, "%s device has connected\n", >> + (speed == USB_SPEED_HIGH) ? "HS" : "FS/LS"); > > unrelated. > >> @@ -179,8 +179,8 @@ static int mxs_phy_on_connect(struct usb_phy *phy, >> static int mxs_phy_on_disconnect(struct usb_phy *phy, >> enum usb_device_speed speed) >> { >> - dev_dbg(phy->dev, "%s speed device has disconnected\n", >> - (speed == USB_SPEED_HIGH) ? "high" : "non-high"); >> + dev_dbg(phy->dev, "%s device has disconnected\n", >> + (speed == USB_SPEED_HIGH) ? "HS" : "FS/LS"); > > unrelated. > Marek suggested using that string, I will added it at another patch. >> @@ -189,6 +189,48 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy, >> return 0; >> } >> >> +static int mxs_phy_on_suspend(struct usb_phy *phy, >> + enum usb_device_speed speed) >> +{ >> + struct mxs_phy *mxs_phy = to_mxs_phy(phy); >> + >> + dev_dbg(phy->dev, "%s device has suspended\n", >> + (speed == USB_SPEED_HIGH) ? "HS" : "FS/LS"); >> + >> + /* delay 4ms to wait bus entering idle */ >> + usleep_range(4000, 5000); >> + >> + if (mxs_phy->data->flags & MXS_PHY_ABNORMAL_IN_SUSPEND) { >> + writel_relaxed(0xffffffff, phy->io_priv + HW_USBPHY_PWD); >> + writel_relaxed(0, phy->io_priv + HW_USBPHY_PWD); >> + } >> + >> + if (speed == USB_SPEED_HIGH) >> + writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT, >> + phy->io_priv + HW_USBPHY_CTRL_CLR); > > why only on HS ? So if !HS and !ABNORMAL, this is no-op. > >> +static int mxs_phy_on_resume(struct usb_phy *phy, >> + enum usb_device_speed speed) >> +{ >> + dev_dbg(phy->dev, "%s device has resumed\n", >> + (speed == USB_SPEED_HIGH) ? "HS" : "FS/LS"); >> + >> + if (speed == USB_SPEED_HIGH) { >> + /* Make sure the device has switched to High-Speed mode */ >> + udelay(500); >> + writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT, >> + phy->io_priv + HW_USBPHY_CTRL_SET); >> + } > > likewise, if !HS it's a no-op. > Correct, this operation is only needed for HS. >> @@ -235,6 +277,11 @@ static int mxs_phy_probe(struct platform_device *pdev) >> >> platform_set_drvdata(pdev, mxs_phy); >> >> + if (mxs_phy->data->flags & MXS_PHY_SENDING_SOF_TOO_FAST) { >> + mxs_phy->phy.notify_suspend = mxs_phy_on_suspend; >> + mxs_phy->phy.notify_resume = mxs_phy_on_resume; >> + } > > hmm, and seems like you only need notify_* on a buggy device. Sorry > Peter but you don't have enough arguments to make me agree with this > (and previous) patch. > > You gotta find a better way to handle this using normal phy > suspend/resume calls. > Like I explained at previous patch, it needs to be notified during ehci suspend/resume. I admit it is a SoC bug, but all SoCs have bugs, hmm. Software needs the solution to workaround it which breaks the standard USB spec. -- BR, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html