On Wed, May 13, 2015 at 10:39:37AM -0500, Bin Liu wrote:
> Felipe,
>
> On 05/13/2015 10:31 AM, Felipe Balbi wrote:
> >Hi,
> >
> >On Wed, May 13, 2015 at 10:18:03AM -0500, Bin Liu wrote:
> >>Felipe,
> >>
> >>On 05/12/2015 05:11 PM, Felipe Balbi wrote:
> >>>As it turns out, all we have to do is set
> >>>Session bit and MUSB will figure out automatically,
> >>>based on ID pin, if it should enter host or
> >>>peripheral roles.
> >>>
> >>>Signed-off-by: Felipe Balbi <balbi@xxxxxx>
> >>>---
> >>> drivers/usb/musb/musb_dsps.c | 43 +++++++++++--------------------------------
> >>> 1 file changed, 11 insertions(+), 32 deletions(-)
> >>>
> >>>diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
> >>>index 659c1099b21f..e7d2d1eb1270 100644
> >>>--- a/drivers/usb/musb/musb_dsps.c
> >>>+++ b/drivers/usb/musb/musb_dsps.c
> >>>@@ -253,45 +253,24 @@ static void otg_timer(unsigned long _musb)
> >>> const struct dsps_musb_wrapper *wrp = glue->wrp;
> >>> u8 devctl;
> >>> unsigned long flags;
> >>>- int skip_session = 0;
> >>>
> >>> /*
> >>> * We poll because DSPS IP's won't expose several OTG-critical
> >>> * status change events (from the transceiver) otherwise.
> >>> */
> >>>- devctl = dsps_readb(mregs, MUSB_DEVCTL);
> >>>- dev_dbg(musb->controller, "Poll devctl %02x (%s)\n", devctl,
> >>>- usb_otg_state_string(musb->xceiv->otg->state));
> >>>-
> >>> spin_lock_irqsave(&musb->lock, flags);
> >>>- switch (musb->xceiv->otg->state) {
> >>>- case OTG_STATE_A_WAIT_BCON:
> >>>- dsps_writeb(musb->mregs, MUSB_DEVCTL, 0);
> >>>- skip_session = 1;
> >>>- /* fall */
> >>>-
> >>>- case OTG_STATE_A_IDLE:
> >>>- case OTG_STATE_B_IDLE:
> >>>- if (devctl & MUSB_DEVCTL_BDEVICE) {
> >>>- musb->xceiv->otg->state = OTG_STATE_B_IDLE;
> >>>- MUSB_DEV_MODE(musb);
> >>>- } else {
> >>>- musb->xceiv->otg->state = OTG_STATE_A_IDLE;
> >>>- MUSB_HST_MODE(musb);
> >>>- }
> >>>- if (!(devctl & MUSB_DEVCTL_SESSION) && !skip_session)
> >>>- dsps_writeb(mregs, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);
> >>>- mod_timer(&glue->timer, jiffies +
> >>>- msecs_to_jiffies(wrp->poll_timeout));
> >>>- break;
> >>>- case OTG_STATE_A_WAIT_VFALL:
> >>>- musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
> >>>- dsps_writel(musb->ctrl_base, wrp->coreintr_set,
> >>>- MUSB_INTR_VBUSERROR << wrp->usb_shift);
> >>>- break;
> >>
> >>This case is for handling VBUSERROR recovery. I am wondering removing
> >>it breaks the recovery.
> >
> >no idea how to test that.
>
> You can remove the VBUS cap, then plug in a thumb drive or hub will most
> likely generate the VBUSERROR interrupt. I think what I did was just short
> VBUS to ground very quickly.
>
> >
> >>>- default:
> >>>- break;
> >>>+ devctl = dsps_readb(mregs, MUSB_DEVCTL);
> >>>+ if (devctl & MUSB_DEVCTL_SESSION) {
> >>>+ goto out;
> >>>+ } else {
> >>>+ devctl |= MUSB_DEVCTL_SESSION;
> >>>+ dsps_writeb(mregs, MUSB_DEVCTL, devctl);
> >>
> >>Is it guaranteed this is for A_IDLE and B_IDLE states? The original
> >>code has otg state check.
> >
> >that check was pointless. This will only do anything when we don't have
> >anything on the other end, so we will always be either in a_idle or
> >b_idle.
>
> It is OK if that was the case. I was wondering if otg_timer() will be fired
> in wrong state which causes SESSION to be set unintentionally.
Yeah, this patch needs more work. The other 3 can be applied safely.
--
balbi
Attachment:
signature.asc
Description: Digital signature
