On Mon, Sep 19, 2016 at 01:44:36PM +0200, Sascha Hauer wrote:
> When switching from gadget to host the driver waits for VBUS

It should be switching from host to gadget

> to disappear before doing the actual switch. This waiting is
> done by letting hw_wait_reg() poll the OTGSC register. This is
> buggy. hw_wait_reg() directly reads the register, but for
> reading the OTGSC register hw_read_otgsc() must be used since
> this function eventually patches the VBUS status read from extcon
> into the raw register value.
> To fix this inline the hw_wait_reg() code into its only user and
> use the correct function to read the OTGSC register. Since
> hw_wait_reg() is unused now remove it.
> 

Stephen Boyd has already posted a similar fix at below:
http://www.spinics.net/lists/linux-usb/msg146304.html

For patch 1, if there is a separate API to wait vbus to be low,
it seems not so necessary.

Peter
> Signed-off-by: Sascha Hauer <s.ha...@pengutronix.de>
> ---
>  drivers/usb/chipidea/ci.h   |  3 ---
>  drivers/usb/chipidea/core.c | 32 --------------------------------
>  drivers/usb/chipidea/otg.c  | 16 ++++++++++++----
>  3 files changed, 12 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index cd41455..05bc4d6 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -428,9 +428,6 @@ int hw_port_test_set(struct ci_hdrc *ci, u8 mode);
>  
>  u8 hw_port_test_get(struct ci_hdrc *ci);
>  
> -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
> -                             u32 value, unsigned int timeout_ms);
> -
>  void ci_platform_configure(struct ci_hdrc *ci);
>  
>  int dbg_create_files(struct ci_hdrc *ci);
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 69426e6..01390e0 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -516,38 +516,6 @@ int hw_device_reset(struct ci_hdrc *ci)
>       return 0;
>  }
>  
> -/**
> - * hw_wait_reg: wait the register value
> - *
> - * Sometimes, it needs to wait register value before going on.
> - * Eg, when switch to device mode, the vbus value should be lower
> - * than OTGSC_BSV before connects to host.
> - *
> - * @ci: the controller
> - * @reg: register index
> - * @mask: mast bit
> - * @value: the bit value to wait
> - * @timeout_ms: timeout in millisecond
> - *
> - * This function returns an error code if timeout
> - */
> -int hw_wait_reg(struct ci_hdrc *ci, enum ci_hw_regs reg, u32 mask,
> -                             u32 value, unsigned int timeout_ms)
> -{
> -     unsigned long elapse = jiffies + msecs_to_jiffies(timeout_ms);
> -
> -     while (hw_read(ci, reg, mask) != value) {
> -             if (time_after(jiffies, elapse)) {
> -                     dev_err(ci->dev, "timeout waiting for %08x in %d\n",
> -                                     mask, reg);
> -                     return -ETIMEDOUT;
> -             }
> -             msleep(20);
> -     }
> -
> -     return 0;
> -}
> -
>  static irqreturn_t ci_irq(int irq, void *data)
>  {
>       struct ci_hdrc *ci = data;
> diff --git a/drivers/usb/chipidea/otg.c b/drivers/usb/chipidea/otg.c
> index 91989be..d1c4cdf 100644
> --- a/drivers/usb/chipidea/otg.c
> +++ b/drivers/usb/chipidea/otg.c
> @@ -104,7 +104,6 @@ void ci_handle_vbus_change(struct ci_hdrc *ci)
>               usb_gadget_vbus_disconnect(&ci->gadget);
>  }
>  
> -#define CI_VBUS_STABLE_TIMEOUT_MS 5000
>  static void ci_handle_id_switch(struct ci_hdrc *ci)
>  {
>       enum ci_role role = ci_otg_role(ci);
> @@ -117,10 +116,19 @@ static void ci_handle_id_switch(struct ci_hdrc *ci)
>  
>       ci_role_stop(ci);
>  
> -     if (role == CI_ROLE_GADGET)
> +     if (role == CI_ROLE_GADGET) {
> +             unsigned long elapse = jiffies + msecs_to_jiffies(5000);
> +
>               /* wait vbus lower than OTGSC_BSV */
> -             hw_wait_reg(ci, OP_OTGSC, OTGSC_BSV, 0,
> -                             CI_VBUS_STABLE_TIMEOUT_MS);
> +             while (hw_read_otgsc(ci, OTGSC_BSV)) {
> +                     if (time_after(jiffies, elapse)) {
> +                             dev_err(ci->dev,
> +                                     "timeout waiting for VBUS disappear\n");
> +                             break;
> +                     }
> +                     msleep(20);
> +             }
> +     }
>  
>       ci_role_start(ci, role);
>  }
> -- 
> 2.8.1
> 
> --
> 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

Reply via email to