On Tue, Oct 16, 2012 at 09:36:46AM +0800, Peter Chen wrote: Hi Felipe & Alex,
Do you have any comments for this patch? > The patch includes both API change and caller change. > The main changes like below: > > - add notify_suspend/notify_resume callback > > This let usb phy driver has the chance to change hw settings during > the controller suspend/resume procedure. > > Besides, old parameter "port" is useless for phy notify, as one usb > phy is only for one usb port. New parameter "speed" stands for > the device's speed which is on the port. > > - implement notify_suspend/notify_resume callback for mxs phy driver > These notify will be called during the bus suspend/resume procedure. > > - Add phy notify at suspend/resume procedure for chipidea host driver > > - refine phy notify operation during connection and disconnection > > The history of this problem like below: > At some i.mx SoCs, when controller works at host mode, the PHY > register needs to be changed at device connect, disconnect, bus > suspend and resume due to the SoC limitations. > > The phy notification should be added according to below rules: > > 1. Only set HW_USBPHY_CTRL.ENHOSTDISCONDETECT > during high speed host mode. > 2. Do not set HW_USBPHY_CTRL.ENHOSTDISCONDETECT > during the reset and speed negotiation period. > 3. Do not set HW_USBPHY_CTRL.ENHOSTDISCONDETECT > during host suspend/resume sequence. > > Please refer: i.mx23RM(page 413) for detail. > http://www.freescale.com/files/dsp/doc/ref_manual/IMX23RM.pdf > > Freescale i.MX SoC, i.mx23, i.mx28 and i.mx6(i.mx6SL does not > need to follow the 3rd rule) need to follow above rules. > > The correct notification setting method should be: > 1. Set connect notify after the second bus reset. > 2. Set disconnect notify after disconnection. > 3. Set suspend nofity after bus goes to suspend (portsc.suspendM=1). > 4. Set resume notify after resume (portsc.fpr=0). > > Signed-off-by: Peter Chen <peter.c...@freescale.com> > Tested-by: Mike Thompson <mpthomp...@gmail.com> > > --- > Changes for v4: > - Delete useless lines which forgot to delete at last patch. > > Changes for v3: > - In order to fix git bitsec error, merge .h and .c to one commit > - The phy notify only needs to be done when udev has existed at hub.c > > Changes for v2: > > - Add Tested-by: Mike Thompson <mpthomp...@gmail.com> > --- > drivers/usb/chipidea/host.c | 75 +++++++++++++++++++++++++++++++++++++++++- > drivers/usb/core/hub.c | 16 ++++---- > drivers/usb/otg/mxs-phy.c | 56 +++++++++++++++++++++++++------- > include/linux/usb/phy.h | 44 +++++++++++++++++++++---- > 4 files changed, 162 insertions(+), 29 deletions(-) > > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c > index ebff9f4..74a6d57 100644 > --- a/drivers/usb/chipidea/host.c > +++ b/drivers/usb/chipidea/host.c > @@ -23,6 +23,7 @@ > #include <linux/usb.h> > #include <linux/usb/hcd.h> > #include <linux/usb/chipidea.h> > +#include <linux/usb/otg.h> > > #define CHIPIDEA_EHCI > #include "../host/ehci-hcd.c" > @@ -46,6 +47,76 @@ static int ci_ehci_setup(struct usb_hcd *hcd) > > return ret; > } > +static enum usb_device_speed ci_get_device_speed(struct ehci_hcd *ehci, > + u32 portsc) > +{ > + if (ehci_is_TDI(ehci)) { > + switch ((portsc >> (ehci->has_hostpc ? 25 : 26)) & 3) { > + case 0: > + return USB_SPEED_FULL; > + case 1: > + return USB_SPEED_LOW; > + case 2: > + return USB_SPEED_HIGH; > + default: > + return USB_SPEED_UNKNOWN; > + } > + } else { > + return USB_SPEED_HIGH; > + } > +} > + > +static int ci_bus_suspend(struct usb_hcd *hcd) > +{ > + int ret; > + struct ehci_hcd *ehci = hcd_to_ehci(hcd); > + int port; > + > + ret = ehci_bus_suspend(hcd); > + if (ret) > + return ret; > + > + port = HCS_N_PORTS(ehci->hcs_params); > + while (port--) { > + u32 __iomem *reg = &ehci->regs->port_status[port]; > + u32 portsc = ehci_readl(ehci, reg); > + > + if (portsc & PORT_CONNECT) { > + enum usb_device_speed speed; > + speed = ci_get_device_speed(ehci, portsc); > + /* notify the USB PHY */ > + if (hcd->phy) > + usb_phy_notify_suspend(hcd->phy, speed); > + } > + } > + > + return ret; > +} > + > +static int ci_bus_resume(struct usb_hcd *hcd) > +{ > + int ret; > + struct ehci_hcd *ehci = hcd_to_ehci(hcd); > + int port; > + > + ret = ehci_bus_resume(hcd); > + > + port = HCS_N_PORTS(ehci->hcs_params); > + while (port--) { > + u32 __iomem *reg = &ehci->regs->port_status[port]; > + u32 portsc = ehci_readl(ehci, reg); > + > + if (portsc & PORT_CONNECT) { > + enum usb_device_speed speed; > + speed = ci_get_device_speed(ehci, portsc); > + /* notify the USB PHY */ > + if (hcd->phy) > + usb_phy_notify_resume(hcd->phy, speed); > + } > + } > + > + return ret; > +} > > static const struct hc_driver ci_ehci_hc_driver = { > .description = "ehci_hcd", > @@ -84,8 +155,8 @@ static const struct hc_driver ci_ehci_hc_driver = { > */ > .hub_status_data = ehci_hub_status_data, > .hub_control = ehci_hub_control, > - .bus_suspend = ehci_bus_suspend, > - .bus_resume = ehci_bus_resume, > + .bus_suspend = ci_bus_suspend, > + .bus_resume = ci_bus_resume, > .relinquish_port = ehci_relinquish_port, > .port_handed_over = ehci_port_handed_over, > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 6dc41c6..7b4062d 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -3994,6 +3994,9 @@ hub_port_init (struct usb_hub *hub, struct usb_device > *udev, int port1, > if (retval) > goto fail; > > + if (hcd->phy && !hdev->parent) > + usb_phy_notify_connect(hcd->phy, udev->speed); > + > /* > * Some superspeed devices have finished the link training process > * and attached to a superspeed hub port, but the device descriptor > @@ -4188,8 +4191,12 @@ static void hub_port_connect_change(struct usb_hub > *hub, int port1, > } > > /* Disconnect any existing devices under this port */ > - if (udev) > + if (udev) { > + if (hcd->phy && !hdev->parent && > + !(portstatus & USB_PORT_STAT_CONNECTION)) > + usb_phy_notify_disconnect(hcd->phy, udev->speed); > usb_disconnect(&hub->ports[port1 - 1]->child); > + } > clear_bit(port1, hub->change_bits); > > /* We can forget about a "removed" device when there's a physical > @@ -4212,13 +4219,6 @@ static void hub_port_connect_change(struct usb_hub > *hub, int port1, > } > } > > - if (hcd->phy && !hdev->parent) { > - if (portstatus & USB_PORT_STAT_CONNECTION) > - usb_phy_notify_connect(hcd->phy, port1); > - else > - usb_phy_notify_disconnect(hcd->phy, port1); > - } > - > /* Return now if debouncing failed or nothing is connected or > * the device was "removed". > */ > diff --git a/drivers/usb/otg/mxs-phy.c b/drivers/usb/otg/mxs-phy.c > index 88db976..41e0543 100644 > --- a/drivers/usb/otg/mxs-phy.c > +++ b/drivers/usb/otg/mxs-phy.c > @@ -96,39 +96,69 @@ static void mxs_phy_enhostdiscondetect_delay(struct > work_struct *ws) > mxs_phy->phy.io_priv + HW_USBPHY_CTRL_SET); > } > > -static int mxs_phy_on_connect(struct usb_phy *phy, int port) > +static int mxs_phy_on_connect(struct usb_phy *phy, > + enum usb_device_speed speed) > { > struct mxs_phy *mxs_phy = to_mxs_phy(phy); > > - dev_dbg(phy->dev, "Connect on port %d\n", port); > - > - mxs_phy_hw_init(mxs_phy); > + dev_dbg(phy->dev, "%s speed device has connected\n", > + (speed == USB_SPEED_HIGH) ? "high" : "non-high"); > > /* > * Delay enabling ENHOSTDISCONDETECT so that connection and > * reset processing can be completed for the root hub. > */ > - dev_dbg(phy->dev, "Delaying setting ENHOSTDISCONDETECT\n"); > - PREPARE_DELAYED_WORK(&mxs_phy->enhostdiscondetect_work, > + if (speed == USB_SPEED_HIGH) { > + PREPARE_DELAYED_WORK(&mxs_phy->enhostdiscondetect_work, > mxs_phy_enhostdiscondetect_delay); > - schedule_delayed_work(&mxs_phy->enhostdiscondetect_work, > + schedule_delayed_work(&mxs_phy->enhostdiscondetect_work, > msecs_to_jiffies(MXY_PHY_ENHOSTDISCONDETECT_DELAY)); > + } > > return 0; > } > > -static int mxs_phy_on_disconnect(struct usb_phy *phy, int port) > +static int mxs_phy_on_disconnect(struct usb_phy *phy, > + enum usb_device_speed speed) > { > - dev_dbg(phy->dev, "Disconnect on port %d\n", port); > + dev_dbg(phy->dev, "%s speed device has disconnected\n", > + (speed == USB_SPEED_HIGH) ? "high" : "non-high"); > > - /* No need to delay before clearing ENHOSTDISCONDETECT. */ > - dev_dbg(phy->dev, "Clearing ENHOSTDISCONDETECT\n"); > - writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT, > + if (speed == USB_SPEED_HIGH) { > + /* No need to delay before clearing ENHOSTDISCONDETECT. */ > + writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT, > + phy->io_priv + HW_USBPHY_CTRL_CLR); > + } > + > + return 0; > +} > + > +static int mxs_phy_on_suspend(struct usb_phy *phy, > + enum usb_device_speed speed) > +{ > + dev_dbg(phy->dev, "At suspend, %s speed device on the port\n", > + (speed == USB_SPEED_HIGH) ? "high" : "non-high"); > + > + if (speed == USB_SPEED_HIGH) > + writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT, > phy->io_priv + HW_USBPHY_CTRL_CLR); > > return 0; > } > > +static int mxs_phy_on_resume(struct usb_phy *phy, > + enum usb_device_speed speed) > +{ > + dev_dbg(phy->dev, "after resume, %s speed device on the port\n", > + (speed == USB_SPEED_HIGH) ? "high" : "non-high"); > + > + if (speed == USB_SPEED_HIGH) > + writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT, > + phy->io_priv + HW_USBPHY_CTRL_SET); > + > + return 0; > +} > + > static int mxs_phy_probe(struct platform_device *pdev) > { > struct resource *res; > @@ -166,6 +196,8 @@ static int mxs_phy_probe(struct platform_device *pdev) > mxs_phy->phy.shutdown = mxs_phy_shutdown; > mxs_phy->phy.notify_connect = mxs_phy_on_connect; > mxs_phy->phy.notify_disconnect = mxs_phy_on_disconnect; > + mxs_phy->phy.notify_suspend = mxs_phy_on_suspend; > + mxs_phy->phy.notify_resume = mxs_phy_on_resume; > > ATOMIC_INIT_NOTIFIER_HEAD(&mxs_phy->phy.notifier); > > diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h > index 06b5bae..45e2235 100644 > --- a/include/linux/usb/phy.h > +++ b/include/linux/usb/phy.h > @@ -10,6 +10,7 @@ > #define __LINUX_USB_PHY_H > > #include <linux/notifier.h> > +#include <linux/usb.h> > > enum usb_phy_events { > USB_EVENT_NONE, /* no events or cable disconnected */ > @@ -98,9 +99,20 @@ struct usb_phy { > int (*set_suspend)(struct usb_phy *x, > int suspend); > > - /* notify phy connect status change */ > - int (*notify_connect)(struct usb_phy *x, int port); > - int (*notify_disconnect)(struct usb_phy *x, int port); > + /* > + * Notify phy that > + * - The controller's connect status change. > + * - The controller's suspend/resume occurs, and the device > + * is on the port. > + */ > + int (*notify_connect)(struct usb_phy *x, > + enum usb_device_speed speed); > + int (*notify_disconnect)(struct usb_phy *x, > + enum usb_device_speed speed); > + int (*notify_suspend)(struct usb_phy *x, > + enum usb_device_speed speed); > + int (*notify_resume)(struct usb_phy *x, > + enum usb_device_speed speed); > }; > > > @@ -189,19 +201,37 @@ usb_phy_set_suspend(struct usb_phy *x, int suspend) > } > > static inline int > -usb_phy_notify_connect(struct usb_phy *x, int port) > +usb_phy_notify_connect(struct usb_phy *x, enum usb_device_speed speed) > { > if (x->notify_connect) > - return x->notify_connect(x, port); > + return x->notify_connect(x, speed); > else > return 0; > } > > static inline int > -usb_phy_notify_disconnect(struct usb_phy *x, int port) > +usb_phy_notify_disconnect(struct usb_phy *x, enum usb_device_speed speed) > { > if (x->notify_disconnect) > - return x->notify_disconnect(x, port); > + return x->notify_disconnect(x, speed); > + else > + return 0; > +} > + > +static inline int > +usb_phy_notify_suspend(struct usb_phy *x, enum usb_device_speed speed) > +{ > + if (x->notify_suspend) > + return x->notify_suspend(x, speed); > + else > + return 0; > +} > + > +static inline int > +usb_phy_notify_resume(struct usb_phy *x, enum usb_device_speed speed) > +{ > + if (x->notify_resume) > + return x->notify_resume(x, speed); > else > return 0; > } > -- > 1.7.0.4 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
