Hello Peter,
On 16-01-14 11:12:32, Peter Chen wrote:
> On Wed, Jan 13, 2016 at 6:12 PM, <maitysanchayan@xxxxxxxxx> wrote:
> > Hello Peter,
> >
> > On 16-01-13 10:33:06, Peter Chen wrote:
> >> On Tue, Jan 12, 2016 at 06:34:47PM +0530, maitysanchayan@xxxxxxxxx wrote:
> >> > Hello Peter,
> >> >
> >> > I had reported the suspend resume issues on Vybrid and recently there
> >> > have been some developments after one of our customers reported the
> >> > below sequence of operations which worked for them.
> >> >
> >> > By doing the following sequence of operations after resume
> >> >
> >> > echo ci_hdrc.1 > /sys/bus/platform/drivers/ci_hdrc/unbind
> >> > echo ci_hdrc.1 > /sys/bus/platform/drivers/ci_hdrc/bind
> >> >
> >> > the USB ports start working. This is observed in presence of external
> >> > hub connected to the root hub or in the presence of root hub alone.
> >> >
> >> > After trying to trace through the sequence of operations which happen
> >> > on bind, roughly are as below,
> >> >
> >> > probe() call in chipidea/core.c
> >> > -> probe() call in hub.c
> >> > ----> hub_configure()
> >> > -------> hub_activate(hub, HUB_INIT)
> >> > ---------> hub_power_on(hub, false)
> >>
> >> Clear PORT_PP (bit 12 at portsc)
> >> > -------------> set_port_feature(hub->hdev, USB_PORT_FEAT_POWER)
> >>
> >> Set PORT_PP (bit 12 at portsc)
> >
> > Sorry it's not clear to me what you are implying below.
> >
> > As in I should implement this in suspend resume path or are you just
> > implying that this is what happens in the above case?
> >
> >> >
> >> > set_port_feature() now sends a control urb. Exactly after this the ports
> >> > start working.
> >> >
> >> > This hub_configure() is not called during the regular resume() sequence.
> >> > I am not well versed with USB specifications or stack to draw an inference
> >> > from this concretely.
> >> >
> >> > Can you throw some light on this if possible? Is it possible, to accomodate
> >> > this somehow in the regular sequence of operations which happen on resume?
> >> > The set_port_feature is in the core usb code and am not sure how to generically
> >> > access it.
> >> >
> >>
> >> Just try to toggle port power to see if it can work for you.
> >
> > Can you clarify how and at what point?
> >
> > At first I assumed you are referring to the fact this could be done from user space
> > as well. So I used a test code to send a usb control msg with request as
> > USB_REQ_(SET/CLEAR)_FEATURE and feature as USB_PORT_FEAT_POWER using libusb and I can
> > control the USB power on/off for any of the ports. This does not work however after
> > resume from suspend.
> >
>
> Hi Sanchayan
>
> I just tried below commands, it will remove the chipidea core device,
> and re-initialize it.
> It means it will reset controller as well as reset PHY. (the
> connection will be lost).
> But we do not reset them during the suspend/resume routine.
>
> >> > echo ci_hdrc.1 > /sys/bus/platform/drivers/ci_hdrc/unbind
> >> > echo ci_hdrc.1 > /sys/bus/platform/drivers/ci_hdrc/bind
>
> If you would like to try if toggle port power can work or not during
> the resume routine,
> try below code please:
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 7404064..00d73c9 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -1195,6 +1195,10 @@ static int ci_resume(struct device *dev)
> if (ret)
> return ret;
>
> + hw_write(ci, OP_PORTSC, BIT(12), ~BIT(12));
> + udelay(10);
> + hw_write(ci, OP_PORTSC, BIT(12), BIT(12));
> +
> if (ci->supports_runtime_pm) {
> pm_runtime_disable(dev);
> pm_runtime_set_active(dev);
>
Do you have any further suggestions that I might try?
I also implemented the below changes which exist in Freescale iMX 3.14.52 kernel,
to see if they work. Earlier I had implemented only the mxs_phy specific code
in the suspend/resume path which did not work. This time I implemented the complete
relevant code.
Of course the following changes did not help either.
Best Regards,
Sanchayan.
diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi
index eb42172..9edc461 100644
--- a/arch/arm/boot/dts/vfxxx.dtsi
+++ b/arch/arm/boot/dts/vfxxx.dtsi
@@ -473,7 +473,7 @@
};
usbdev0: usb@40034000 {
- compatible = "fsl,vf610-usb", "fsl,imx27-usb";
+ compatible = "fsl,vf610-usb";
reg = <0x40034000 0x800>;
interrupts = <75 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks VF610_CLK_USBC0>;
@@ -608,7 +608,7 @@
};
usbh1: usb@400b4000 {
- compatible = "fsl,vf610-usb", "fsl,imx27-usb";
+ compatible = "fsl,vf610-usb";
reg = <0x400b4000 0x800>;
interrupts = <76 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&clks VF610_CLK_USBC1>;
diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c b/drivers/usb/chipidea/ci_hdrc_imx.c
index df1ad5e..e0e6800 100644
--- a/drivers/usb/chipidea/ci_hdrc_imx.c
+++ b/drivers/usb/chipidea/ci_hdrc_imx.c
@@ -51,12 +51,17 @@ static const struct ci_hdrc_imx_platform_flag imx6sx_usb_data = {
CI_HDRC_TURN_VBUS_EARLY_ON,
};
+static const struct ci_hdrc_imx_platform_flag vf610_usb_data = {
+ .flags = CI_HDRC_IMX_EHCI_QUIRK,
+};
+
static const struct of_device_id ci_hdrc_imx_dt_ids[] = {
{ .compatible = "fsl,imx28-usb", .data = &imx28_usb_data},
{ .compatible = "fsl,imx27-usb", .data = &imx27_usb_data},
{ .compatible = "fsl,imx6q-usb", .data = &imx6q_usb_data},
{ .compatible = "fsl,imx6sl-usb", .data = &imx6sl_usb_data},
{ .compatible = "fsl,imx6sx-usb", .data = &imx6sx_usb_data},
+ { .compatible = "fsl,vf610-usb", .data = &vf610_usb_data},
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, ci_hdrc_imx_dt_ids);
diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
index 2f8af40..6051826 100644
--- a/drivers/usb/chipidea/host.c
+++ b/drivers/usb/chipidea/host.c
@@ -34,11 +34,29 @@
static struct hc_driver __read_mostly ci_ehci_hc_driver;
static int (*orig_bus_suspend)(struct usb_hcd *hcd);
+static int (*orig_bus_resume)(struct usb_hcd *hcd);
+static int (*orig_hub_control)(struct usb_hcd *hcd,
+ u16 typeReq, u16 wValue, u16 wIndex,
+ char *buf, u16 wLength);
struct ehci_ci_priv {
struct regulator *reg_vbus;
};
+/* This function is used to override WKCN, WKDN, and WKOC */
+static void ci_ehci_override_wakeup_flag(struct ehci_hcd *ehci,
+ u32 __iomem *reg, u32 flags, bool set)
+{
+ u32 val = ehci_readl(ehci, reg);
+
+ if (set)
+ val |= flags;
+ else
+ val &= ~flags;
+
+ ehci_writel(ehci, val, reg);
+}
+
static int ehci_ci_portpower(struct usb_hcd *hcd, int portnum, bool enable)
{
struct ehci_hcd *ehci = hcd_to_ehci(hcd);
@@ -72,6 +90,125 @@ static const struct ehci_driver_overrides ehci_ci_overrides = {
.port_power = ehci_ci_portpower,
};
+static int ci_imx_ehci_bus_resume(struct usb_hcd *hcd)
+{
+ struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+ int port;
+
+ int ret = orig_bus_resume(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);
+ /*
+ * Notify PHY after resume signal has finished, it is
+ * for global suspend case.
+ */
+ if (hcd->usb_phy
+ && test_bit(port, &ehci->bus_suspended)
+ && (portsc & PORT_CONNECT)
+ && (ehci_port_speed(ehci, portsc) ==
+ USB_PORT_STAT_HIGH_SPEED))
+ /* notify the USB PHY */
+ usb_phy_notify_resume(hcd->usb_phy, USB_SPEED_HIGH);
+ }
+
+ return 0;
+}
+
+static int ci_imx_ehci_hub_control(
+ struct usb_hcd *hcd,
+ u16 typeReq,
+ u16 wValue,
+ u16 wIndex,
+ char *buf,
+ u16 wLength
+)
+{
+ struct ehci_hcd *ehci = hcd_to_ehci(hcd);
+ u32 __iomem *status_reg;
+ u32 temp;
+ unsigned long flags;
+ int retval = 0;
+ struct device *dev = hcd->self.controller;
+ struct ci_hdrc *ci = dev_get_drvdata(dev);
+
+ status_reg = &ehci->regs->port_status[(wIndex & 0xff) - 1];
+
+ spin_lock_irqsave(&ehci->lock, flags);
+
+ if (typeReq == SetPortFeature && wValue == USB_PORT_FEAT_SUSPEND) {
+ temp = ehci_readl(ehci, status_reg);
+ if ((temp & PORT_PE) == 0 || (temp & PORT_RESET) != 0) {
+ retval = -EPIPE;
+ goto done;
+ }
+
+ temp &= ~(PORT_RWC_BITS | PORT_WKCONN_E);
+ temp |= PORT_WKDISC_E | PORT_WKOC_E;
+ ehci_writel(ehci, temp | PORT_SUSPEND, status_reg);
+
+ /*
+ * If a transaction is in progress, there may be a delay in
+ * suspending the port. Poll until the port is suspended.
+ */
+ if (ehci_handshake(ehci, status_reg, PORT_SUSPEND,
+ PORT_SUSPEND, 5000))
+ ehci_err(ehci, "timeout waiting for SUSPEND\n");
+
+ if (ci->platdata->flags & CI_HDRC_IMX_IS_HSIC) {
+ if (ci->platdata->notify_event)
+ ci->platdata->notify_event
+ (ci, CI_HDRC_IMX_HSIC_SUSPEND_EVENT);
+ ci_ehci_override_wakeup_flag(ehci, status_reg,
+ PORT_WKDISC_E | PORT_WKCONN_E, false);
+ }
+
+ spin_unlock_irqrestore(&ehci->lock, flags);
+ if (ehci_port_speed(ehci, temp) ==
+ USB_PORT_STAT_HIGH_SPEED && hcd->usb_phy) {
+ /* notify the USB PHY */
+ usb_phy_notify_suspend(hcd->usb_phy, USB_SPEED_HIGH);
+ }
+ spin_lock_irqsave(&ehci->lock, flags);
+
+ set_bit((wIndex & 0xff) - 1, &ehci->suspended_ports);
+ goto done;
+ }
+
+ /*
+ * After resume has finished, it needs do some post resume
+ * operation for some SoCs.
+ */
+ else if (typeReq == ClearPortFeature &&
+ wValue == USB_PORT_FEAT_C_SUSPEND) {
+
+ /* Make sure the resume has finished, it should be finished */
+ if (ehci_handshake(ehci, status_reg, PORT_RESUME, 0, 25000))
+ ehci_err(ehci, "timeout waiting for resume\n");
+
+ temp = ehci_readl(ehci, status_reg);
+
+ if (ehci_port_speed(ehci, temp) ==
+ USB_PORT_STAT_HIGH_SPEED && hcd->usb_phy) {
+ /* notify the USB PHY */
+ usb_phy_notify_resume(hcd->usb_phy, USB_SPEED_HIGH);
+ }
+ }
+
+ spin_unlock_irqrestore(&ehci->lock, flags);
+
+ /* Handle the hub control events here */
+ return orig_hub_control(hcd, typeReq, wValue, wIndex, buf, wLength);
+done:
+ spin_unlock_irqrestore(&ehci->lock, flags);
+ return retval;
+}
+
static irqreturn_t host_irq(struct ci_hdrc *ci)
{
return usb_hcd_irq(ci->irq, ci->hcd);
@@ -184,6 +321,8 @@ static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
struct ehci_hcd *ehci = hcd_to_ehci(hcd);
int port;
u32 tmp;
+ struct device *dev = hcd->self.controller;
+ struct ci_hdrc *ci = dev_get_drvdata(dev);
int ret = orig_bus_suspend(hcd);
@@ -213,6 +352,29 @@ static int ci_ehci_bus_suspend(struct usb_hcd *hcd)
* It needs a short delay between set RS bit and PHCD.
*/
usleep_range(150, 200);
+
+ /*
+ * If a transaction is in progress, there may be a delay in
+ * suspending the port. Poll until the port is suspended.
+ */
+ if (test_bit(port, &ehci->bus_suspended) &&
+ ehci_handshake(ehci, reg, PORT_SUSPEND,
+ PORT_SUSPEND, 5000))
+ ehci_err(ehci, "timeout waiting for SUSPEND\n");
+
+ if (ci->platdata->flags & CI_HDRC_IMX_IS_HSIC)
+ ci_ehci_override_wakeup_flag(ehci, reg,
+ PORT_WKDISC_E | PORT_WKCONN_E, false);
+
+ if (hcd->usb_phy && test_bit(port, &ehci->bus_suspended)
+ && (ehci_port_speed(ehci, portsc) ==
+ USB_PORT_STAT_HIGH_SPEED))
+ /*
+ * notify the USB PHY, it is for global
+ * suspend case.
+ */
+ usb_phy_notify_suspend(hcd->usb_phy,
+ USB_SPEED_HIGH);
break;
}
}
@@ -237,6 +399,17 @@ int ci_hdrc_host_init(struct ci_hdrc *ci)
rdrv->name = "host";
ci->roles[CI_ROLE_HOST] = rdrv;
+ ehci_init_driver(&ci_ehci_hc_driver, &ehci_ci_overrides);
+ orig_bus_suspend = ci_ehci_hc_driver.bus_suspend;
+ orig_bus_resume = ci_ehci_hc_driver.bus_resume;
+ orig_hub_control = ci_ehci_hc_driver.hub_control;
+
+ ci_ehci_hc_driver.bus_suspend = ci_ehci_bus_suspend;
+ if (ci->platdata->flags & CI_HDRC_IMX_EHCI_QUIRK) {
+ ci_ehci_hc_driver.bus_resume = ci_imx_ehci_bus_resume;
+ ci_ehci_hc_driver.hub_control = ci_imx_ehci_hub_control;
+ }
+
return 0;
}
diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 3fcc048..aa53f6c 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -135,7 +135,8 @@ static const struct mxs_phy_data imx6sl_phy_data = {
};
static const struct mxs_phy_data vf610_phy_data = {
- .flags = MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
+ .flags = MXS_PHY_ABNORMAL_IN_SUSPEND | MXS_PHY_SENDING_SOF_TOO_FAST |
+ MXS_PHY_DISCONNECT_LINE_WITHOUT_VBUS |
MXS_PHY_NEED_IP_FIX,
};
@@ -445,6 +446,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(0xffffffff, phy->io_priv + HW_USBPHY_PWD);
+ writel(0, phy->io_priv + HW_USBPHY_PWD);
+ }
+
+ if (speed == USB_SPEED_HIGH)
+ writel(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
+ phy->io_priv + HW_USBPHY_CTRL_CLR);
+
+ return 0;
+}
+
+/*
+ * The resume signal must be finished here.
+ */
+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(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;
@@ -498,6 +541,10 @@ static int mxs_phy_probe(struct platform_device *pdev)
mxs_phy->phy.notify_disconnect = mxs_phy_on_disconnect;
mxs_phy->phy.type = USB_PHY_TYPE_USB2;
mxs_phy->phy.set_wakeup = mxs_phy_set_wakeup;
+ /* 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;
+ /* } */
mxs_phy->clk = clk;
mxs_phy->data = of_id->data;
diff --git a/include/linux/usb/chipidea.h b/include/linux/usb/chipidea.h
index c22f68b..733c6e8 100644
--- a/include/linux/usb/chipidea.h
+++ b/include/linux/usb/chipidea.h
@@ -29,9 +29,12 @@ struct ci_hdrc_platform_data {
#define CI_HDRC_IMX28_WRITE_FIX BIT(5)
#define CI_HDRC_FORCE_FULLSPEED BIT(6)
#define CI_HDRC_TURN_VBUS_EARLY_ON BIT(7)
+#define CI_HDRC_IMX_EHCI_QUIRK BIT(8)
+#define CI_HDRC_IMX_IS_HSIC BIT(9)
enum usb_dr_mode dr_mode;
#define CI_HDRC_CONTROLLER_RESET_EVENT 0
#define CI_HDRC_CONTROLLER_STOPPED_EVENT 1
+#define CI_HDRC_IMX_HSIC_SUSPEND_EVENT 2
void (*notify_event) (struct ci_hdrc *ci, unsigned event);
struct regulator *reg_vbus;
bool tpl_support;
diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
index bc91b5d..4bc70f4 100644
--- a/include/linux/usb/phy.h
+++ b/include/linux/usb/phy.h
@@ -122,6 +122,10 @@ struct usb_phy {
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);
};
/**
@@ -302,6 +306,24 @@ usb_phy_notify_disconnect(struct usb_phy *x, enum usb_device_speed speed)
return 0;
}
+static inline int usb_phy_notify_suspend
+ (struct usb_phy *x, enum usb_device_speed speed)
+{
+ if (x && 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 && x->notify_resume)
+ return x->notify_resume(x, speed);
+ else
+ return 0;
+}
+
/* notifiers */
static inline int
usb_register_notifier(struct usb_phy *x, struct notifier_block *nb)
