Re: [PATCH 07/15] usb: musb: Handle cable status better for 2430 glue layer

Bin Liu
2016-05-13T19:57:00Z

Hi,

On Wed, May 11, 2016 at 06:33:04PM -0700, Tony Lindgren wrote:
> * Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> [160511 17:56]:
> > We may have drivers loaded but no configured gadgets and MUSB may be in
> > host mode. If gadgets are configured during host mode, PM runtime will
> > get confused.
> ...
>
> > @@ -307,6 +350,9 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
> > dev_dbg(dev, "ID float\n");
> > }
> >
> > + if (!glue->cable_connected)
> > + omap2430_set_power(musb, glue->enabled, cable_connected);
> > +
>
> Oops sorry I messed up while cleaning up. This should be just cable_connected
> here instead of glue->cable_connected. Otherwise we won't idle on cable
> unplug as we're always using the cached value instead of the current
> value.
>
> Updated patch below.
>
> Regards,
>
> Tony
>
> 8< -----------------
> From: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> Date: Mon, 9 May 2016 07:50:57 -0700
> Subject: [PATCH] usb: musb: Handle cable status better for 2430 glue layer
>
> We may have drivers loaded but no configured gadgets and MUSB may be in
> host mode. If gadgets are configured during host mode, PM runtime will
> get confused.
>
> Disable PM runtime from gadget state, and do it based on the cable
> and last state.
>
> Note that we may get multiple cable events, so we need to keep track
> of the power state.
>
> Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
>
> --- a/drivers/usb/musb/omap2430.c
> +++ b/drivers/usb/musb/omap2430.c
> @@ -49,6 +49,9 @@ struct omap2430_glue {
> enum musb_vbus_id_status status;
> struct work_struct omap_musb_mailbox_work;
> struct device *control_otghs;
> + bool cable_connected;
> + bool enabled;
> + bool powered;

This variable is only used within omap2430_set_power(), so it can be
local within that function to save a few bytes.

Regards,
-Bin.

> };
> #define glue_to_musb(g) platform_get_drvdata(g->musb)
>
> @@ -234,6 +237,45 @@ static inline void omap2430_low_level_init(struct musb *musb)
> musb_writel(musb->mregs, OTG_FORCESTDBY, l);
> }
>
> +/*
> + * We can get multiple cable events so we need to keep track
> + * of the power state. Only keep power enabled if USB cable is
> + * connected and a gadget is started.
> + */
> +static void omap2430_set_power(struct musb *musb, bool enabled, bool cable)
> +{
> + struct device *dev = musb->controller;
> + struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
> + bool power_up;
> + int res;
> +
> + if (glue->enabled != enabled)
> + glue->enabled = enabled;
> +
> + if (glue->cable_connected != cable)
> + glue->cable_connected = cable;
> +
> + power_up = glue->enabled && glue->cable_connected;
> + if (power_up == glue->powered) {
> + dev_warn(musb->controller, "power state already %i\n",
> + power_up);
> + return;
> + }
> +
> + glue->powered = power_up;
> +
> + if (power_up) {
> + res = pm_runtime_get_sync(musb->controller);
> + if (res < 0) {
> + dev_err(musb->controller, "could not enable: %i", res);
> + glue->powered = false;
> + }
> + } else {
> + pm_runtime_mark_last_busy(musb->controller);
> + pm_runtime_put_autosuspend(musb->controller);
> + }
> +}
> +
> static void omap2430_musb_mailbox(enum musb_vbus_id_status status)
> {
> struct omap2430_glue *glue = _glue;
> @@ -259,6 +301,13 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
> struct musb_hdrc_platform_data *pdata = dev_get_platdata(dev);
> struct omap_musb_board_data *data = pdata->board_data;
> struct usb_otg *otg = musb->xceiv->otg;
> + bool cable_connected;
> +
> + cable_connected = ((glue->status == MUSB_ID_GROUND) ||
> + (glue->status == MUSB_VBUS_VALID));
> +
> + if (cable_connected)
> + omap2430_set_power(musb, glue->enabled, cable_connected);
>
> switch (glue->status) {
> case MUSB_ID_GROUND:
> @@ -268,7 +317,6 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
> musb->xceiv->otg->state = OTG_STATE_A_IDLE;
> musb->xceiv->last_event = USB_EVENT_ID;
> if (musb->gadget_driver) {
> - pm_runtime_get_sync(dev);
> omap_control_usb_set_mode(glue->control_otghs,
> USB_MODE_HOST);
> omap2430_musb_set_vbus(musb, 1);
> @@ -281,8 +329,6 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
> otg->default_a = false;
> musb->xceiv->otg->state = OTG_STATE_B_IDLE;
> musb->xceiv->last_event = USB_EVENT_VBUS;
> - if (musb->gadget_driver)
> - pm_runtime_get_sync(dev);
> omap_control_usb_set_mode(glue->control_otghs, USB_MODE_DEVICE);
> break;
>
> @@ -291,11 +337,8 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
> dev_dbg(dev, "VBUS Disconnect\n");
>
> musb->xceiv->last_event = USB_EVENT_NONE;
> - if (musb->gadget_driver) {
> + if (musb->gadget_driver)
> omap2430_musb_set_vbus(musb, 0);
> - pm_runtime_mark_last_busy(dev);
> - pm_runtime_put_autosuspend(dev);
> - }
>
> if (data->interface_type == MUSB_INTERFACE_UTMI)
> otg_set_vbus(musb->xceiv->otg, 0);
> @@ -307,6 +350,9 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue)
> dev_dbg(dev, "ID float\n");
> }
>
> + if (!cable_connected)
> + omap2430_set_power(musb, glue->enabled, cable_connected);
> +
> atomic_notifier_call_chain(&musb->xceiv->notifier,
> musb->xceiv->last_event, NULL);
> }
> @@ -443,6 +489,8 @@ static void omap2430_musb_enable(struct musb *musb)
> struct musb_hdrc_platform_data *pdata = dev_get_platdata(dev);
> struct omap_musb_board_data *data = pdata->board_data;
>
> + omap2430_set_power(musb, true, glue->cable_connected);
> +
> switch (glue->status) {
>
> case MUSB_ID_GROUND:
> @@ -481,6 +529,8 @@ static void omap2430_musb_disable(struct musb *musb)
> if (glue->status != MUSB_UNKNOWN)
> omap_control_usb_set_mode(glue->control_otghs,
> USB_MODE_DISCONNECT);
> +
> + omap2430_set_power(musb, false, glue->cable_connected);
> }
>
> static int omap2430_musb_exit(struct musb *musb)
> @@ -653,11 +703,13 @@ err0:
>
> static int omap2430_remove(struct platform_device *pdev)
> {
> - struct omap2430_glue *glue = platform_get_drvdata(pdev);
> + struct omap2430_glue *glue = platform_get_drvdata(pdev);
> + struct musb *musb = glue_to_musb(glue);
>
> pm_runtime_get_sync(glue->dev);
> cancel_work_sync(&glue->omap_musb_mailbox_work);
> platform_device_unregister(glue->musb);
> + omap2430_set_power(musb, false, false);
> pm_runtime_put_sync(glue->dev);
> pm_runtime_disable(glue->dev);