>-----Original Message----- >From: Felipe Balbi [mailto:felipe.balbi@xxxxxxxxx] >Sent: Friday, January 29, 2010 6:13 PM >To: ext Mike Frysinger >Cc: linux-usb@xxxxxxxxxxxxxxx; Balbi Felipe >(Nokia-D/Helsinki); uclinux-dist-devel@xxxxxxxxxxxxxxxxxxxx; Cai, Cliff >Subject: Re: [PATCH] USB: musb: support host/gadget role >switching on Blackfin parts > >On Fri, Jan 29, 2010 at 02:56:15AM +0100, ext Mike Frysinger wrote: >>From: Cliff Cai <cliff.cai@xxxxxxxxxx> > >please, always put a description here ;-) > >>@@ -173,6 +173,13 @@ static irqreturn_t >blackfin_interrupt(int irq, void *__hci) >> retval = musb_interrupt(musb); >> } >> >>+ /* Start sampling ID pin, when plug is removed from MUSB */ >>+ if (is_otg_enabled(musb) && (musb->xceiv->state == >OTG_STATE_B_IDLE >>+ || musb->xceiv->state == OTG_STATE_A_WAIT_BCON)) { >>+ mod_timer(&musb_conn_timer, jiffies + TIMER_DELAY); > >you sample id pin with a timer ? Can't you get that info from >your transceiver ? No irq ?? Unfortunately,if musb is in host mode,it can't be recognized by host(PC), While if it's in peripheral mode,it can't detect the inserting of device. So,we have to use a timer do start detecting the inserted plug type. >>@@ -196,10 +204,44 @@ static void >musb_conn_timer_handler(unsigned long _musb) >> case OTG_STATE_A_WAIT_BCON: >> /* Start a new session */ >> val = musb_readw(musb->mregs, MUSB_DEVCTL); >>+ val &= ~MUSB_DEVCTL_SESSION; >>+ musb_writew(musb->mregs, MUSB_DEVCTL, val); >>+ val = musb_readw(musb->mregs, MUSB_DEVCTL); >> val |= MUSB_DEVCTL_SESSION; >> musb_writew(musb->mregs, MUSB_DEVCTL, val); >>+ val = musb_readw(musb->mregs, MUSB_DEVCTL); > >you read the same DEVCTL register over and over again. Read it >once and only change the bits you want to change: >val = musb_readw(musb->mregs, MUSB_DEVCTL); val &= >~MUSB_DEVCTL_SESSION; musb_writew(musb->mregs, MUSB_DEVCTL, >val); val |= MUSB_DEVCTL_SESSION; musb_writew(musb->mregs, >MUSB_DEVCTL, val); > >btw, why do you disable and right after reenable session bit ? >A little comment would make things clearer, is that really necessary ? It's really necessary,or if musb is in host mode,then it can't detect the B-plug If it's connected to PC. > >>+ if (!(val & MUSB_DEVCTL_BDEVICE)) { > >if you're in A_WAIT_BCON, this is actually the only >possibility, if not, then your state handling is kinda screwed up :-p > >>+ gpio_set_value(musb->config->gpio_vrsel, 1); >>+ musb->xceiv->state = OTG_STATE_A_WAIT_BCON; >>+ } else { > >so this would be done if we're BDEVICE, right ? then it >shouldn't be done here. Actually,after a peripheral was removed from musb,msub is still in A_WAIT_BCON,but it's possible that Musb is going to be connected to PC,so,we need to deal with such scenario. >>+ gpio_set_value(musb->config->gpio_vrsel, 0); >>+ /* Ignore VBUSERROR and SUSPEND IRQ */ >>+ val = musb_readb(musb->mregs, MUSB_INTRUSBE); >>+ val &= ~MUSB_INTR_VBUSERROR; >>+ musb_writeb(musb->mregs, MUSB_INTRUSBE, val); >>+ >>+ val = MUSB_INTR_SUSPEND | MUSB_INTR_VBUSERROR; >>+ musb_writeb(musb->mregs, MUSB_INTRUSB, val); >>+ if (is_otg_enabled(musb)) >>+ musb->xceiv->state = OTG_STATE_B_IDLE; >>+ else >>+ musb_writeb(musb->mregs, >MUSB_POWER, MUSB_POWER_HSENAB); >>+ } >>+ mod_timer(&musb_conn_timer, jiffies + TIMER_DELAY); >>+ break; >>+ case OTG_STATE_B_IDLE: >> >>+ if (!is_peripheral_enabled(musb)) >>+ break; >>+ /* Start a new session. It seems that MUSB needs taking >>+ * some time to recognize the type of the plug inserted? >>+ */ >>+ val = musb_readw(musb->mregs, MUSB_DEVCTL); >>+ val |= MUSB_DEVCTL_SESSION; >>+ musb_writew(musb->mregs, MUSB_DEVCTL, val); >> val = musb_readw(musb->mregs, MUSB_DEVCTL); >>+ >> if (!(val & MUSB_DEVCTL_BDEVICE)) { > >if !B == A, we can't be in any A_* states here. It seems not reasonable to turn to A_WAIT_B,but we are performing role switching not real otg... This code is used to deal with the case that after musb was removed from PC then a peripheral device is inserted into it. At this point musb should turn to A_* state for the further state transition. Cliff -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html