On Thu, Feb 26, 2015 at 2:19 PM, Felipe Balbi <ba...@ti.com> wrote: > Hi again, > > On Thu, Feb 26, 2015 at 02:15:40PM -0600, Felipe Balbi wrote: >> > > >> >> > On Thu, Feb 26, 2015 at 12:25:16PM -0600, Felipe Balbi wrote: >> > > >> >> >> FSDEV is set for both HIGH and FULL speeds, >> > > >> >> >> the correct HIGHSPEED check is done through >> > > >> >> >> power register's HSMODE bit. >> > > >> >> >> >> > > >> >> >> Signed-off-by: Felipe Balbi <ba...@ti.com> >> > > >> >> > >> > > >> >> > I'm still unsure if we should really ignore babble on FS/LS. It >> > > >> >> > seems to >> > > >> >> > me we should never ignore it, but I really don't have a way to >> > > >> >> > prove >> > > >> >> > this statement. For the sake of reducing impact, we will just >> > > >> >> > fix HS >> > > >> >> > check here. >> > > >> >> > >> > > >> >> >> > > >> >> I believe we should drop speed check in here and not ignore babble >> > > >> >> regardless. We have seen many cases that full-speed babble causes >> > > >> >> MUSB >> > > >> >> stop working. >> > > >> > >> > > >> > I'll make that as a separate patch then, just to make sure we can >> > > >> > revert >> > > >> > it later if something goes wrong ;-) >> > > >> >> > > >> Agreed. >> > > > >> > > > I noticed something else. If we really don't need to reset musb in case >> > > > of babble, then we can drop that recover_work completely which >> > > > simplifies babble handling quite a bit. >> > > > >> > > > I'll fiddle with that, if you don't mind. >> > > >> > > That is fine with me. I am writing the comments for the dropping reset >> > > patch right now ;) >> > > >> > > We only need the following in musb_recover_work() for the recovery. >> > > >> > > 1852 /* >> > > 1853 * When a babble condition occurs, the musb controller >> > > 1854 * removes the session bit and the endpoint config is lost. >> > > 1855 */ >> > > 1856 if (musb->dyn_fifo) >> > > 1857 status = ep_config_from_table(musb); >> > > 1858 else >> > > 1859 status = ep_config_from_hw(musb); >> > > 1860 >> > > 1861 /* start the session again */ >> > > 1862 if (status == 0) >> > > 1863 musb_start(musb); >> > >> > the statement that musb looses ep configuration seems bogus to me. I >> > dropped that too: >> > >> > commit 4a07d415bf5894c66f61827c30053a65d6dfce26 >> > Author: Felipe Balbi <ba...@ti.com> >> > Date: Thu Feb 26 14:02:35 2015 -0600 >> > >> > usb: musb: core: simplify musb_recover_work() >> > >> > we're not resetting musb at all, just restarting >> > the session. This means we don't need to touch PHYs >> > or VBUS or anything like that. Just make sure session >> > bit is reenabled after MUSB dropped it. >> > >> > Signed-off-by: Felipe Balbi <ba...@ti.com> >> > >> > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c >> > index d9c627d54db6..219636c1b020 100644 >> > --- a/drivers/usb/musb/musb_core.c >> > +++ b/drivers/usb/musb/musb_core.c >> > @@ -1835,7 +1835,8 @@ static void musb_irq_work(struct work_struct *data) >> > static void musb_recover_work(struct work_struct *data) >> > { >> > struct musb *musb = container_of(data, struct musb, recover_work.work); >> > - int status, ret; >> > + int ret; >> > + u8 devctl; >> > >> > ret = musb_platform_reset(musb); >> > if (ret) { >> > @@ -1843,24 +1844,17 @@ static void musb_recover_work(struct work_struct >> > *data) >> > return; >> > } >> > >> > - usb_phy_vbus_off(musb->xceiv); >> > - usleep_range(100, 200); >> > - >> > - usb_phy_vbus_on(musb->xceiv); >> > - usleep_range(100, 200); >> > + /* drop session bit */ >> > + devctl = musb_readb(musb->mregs, MUSB_DEVCTL); >> > + devctl &= ~MUSB_DEVCTL_SESSION; >> > + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl); >> > >> > - /* >> > - * When a babble condition occurs, the musb controller >> > - * removes the session bit and the endpoint config is lost. >> > - */ >> > - if (musb->dyn_fifo) >> > - status = ep_config_from_table(musb); >> > - else >> > - status = ep_config_from_hw(musb); >> > + /* wait for session to really drop */ >> > + udelay(100); >> > >> > - /* start the session again */ >> > - if (status == 0) >> > - musb_start(musb); >> > + /* restart session */ >> > + devctl |= MUSB_DEVCTL_SESSION; >> > + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl); >> > } >> > >> > /* >> > -------------------------------------------------------------------------- >> > >> > >> > the only thing that we're still missing is a notification to usbcore >> > that we have a port being disabled, that's the only way to have testusb >> > know that it has been disconnected by the host. >> >> here's the final version: >> >> commit 3db11e7da5938bbd955410355194625f699a424c >> Author: Felipe Balbi <ba...@ti.com> >> Date: Thu Feb 26 14:02:35 2015 -0600 >> >> usb: musb: core: simplify musb_recover_work() >> >> we're not resetting musb at all, just restarting >> the session. This means we don't need to touch PHYs >> or VBUS or anything like that. Just make sure session >> bit is reenabled after MUSB dropped it. >> >> while at that, make sure to tell usbcore that we're >> dropping the session and, thus, disconnecting the >> device. >> >> Signed-off-by: Felipe Balbi <ba...@ti.com> >> >> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c >> index d9c627d54db6..64cb242d92f7 100644 >> --- a/drivers/usb/musb/musb_core.c >> +++ b/drivers/usb/musb/musb_core.c >> @@ -1835,7 +1835,8 @@ static void musb_irq_work(struct work_struct *data) >> static void musb_recover_work(struct work_struct *data) >> { >> struct musb *musb = container_of(data, struct musb, recover_work.work); >> - int status, ret; >> + int ret; >> + u8 devctl; >> >> ret = musb_platform_reset(musb); >> if (ret) { >> @@ -1843,24 +1844,17 @@ static void musb_recover_work(struct work_struct >> *data) >> return; >> } >> >> - usb_phy_vbus_off(musb->xceiv); >> - usleep_range(100, 200); >> - >> - usb_phy_vbus_on(musb->xceiv); >> - usleep_range(100, 200); >> + /* drop session bit */ >> + devctl = musb_readb(musb->mregs, MUSB_DEVCTL); >> + devctl &= ~MUSB_DEVCTL_SESSION; >> + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl); >> >> - /* >> - * When a babble condition occurs, the musb controller >> - * removes the session bit and the endpoint config is lost. >> - */ >> - if (musb->dyn_fifo) >> - status = ep_config_from_table(musb); >> - else >> - status = ep_config_from_hw(musb); >> + /* tell usbcore about it */ >> + musb_root_disconnect(musb); >> >> - /* start the session again */ >> - if (status == 0) >> - musb_start(musb); >> + /* restart session */ >> + devctl |= MUSB_DEVCTL_SESSION; >> + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl); >> } >> >> /* >> -------------------------------------------------------------------------- >> >> now I'll move all of this inside the irq handler and rename >> musb_platform_reset() to musb_platform_recover_from_babble() > > it looks like we really do need to redo fifo sizing. So here's a version > which I tested and can see that even after babble, device reenumerates. > > commit 1b9f864804aab6b250084b7adab9b520903f0a1d > Author: Felipe Balbi <ba...@ti.com> > Date: Thu Feb 26 14:02:35 2015 -0600 > > usb: musb: core: simplify musb_recover_work() > > we're not resetting musb at all, just restarting > the session. This means we don't need to touch PHYs > or VBUS or anything like that. Just make sure session > bit is reenabled after MUSB dropped it. > > while at that, make sure to tell usbcore that we're > dropping the session and, thus, disconnecting the > device. > > Signed-off-by: Felipe Balbi <ba...@ti.com> > > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > index d9c627d54db6..1ae4c3c28d68 100644 > --- a/drivers/usb/musb/musb_core.c > +++ b/drivers/usb/musb/musb_core.c > @@ -1835,7 +1835,8 @@ static void musb_irq_work(struct work_struct *data) > static void musb_recover_work(struct work_struct *data) > { > struct musb *musb = container_of(data, struct musb, > recover_work.work); > - int status, ret; > + int ret; > + u8 devctl; > > ret = musb_platform_reset(musb); > if (ret) { > @@ -1843,23 +1844,25 @@ static void musb_recover_work(struct work_struct > *data) > return; > } > > - usb_phy_vbus_off(musb->xceiv); > - usleep_range(100, 200); > + /* drop session bit */ > + devctl = musb_readb(musb->mregs, MUSB_DEVCTL); > + devctl &= ~MUSB_DEVCTL_SESSION; > + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl); > > - usb_phy_vbus_on(musb->xceiv); > - usleep_range(100, 200); > + /* tell usbcore about it */ > + musb_root_disconnect(musb); > > /* > * When a babble condition occurs, the musb controller > * removes the session bit and the endpoint config is lost. > */ > if (musb->dyn_fifo) > - status = ep_config_from_table(musb); > + ret = ep_config_from_table(musb); > else > - status = ep_config_from_hw(musb); > + ret = ep_config_from_hw(musb); > > - /* start the session again */ > - if (status == 0) > + /* restart session */ > + if (ret == 0) > musb_start(musb); > } >
Looks good to me. You also need the following to not reset MUSB. diff --git a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c index b52eeca..4da8388 100644 --- a/drivers/usb/musb/musb_dsps.c +++ b/drivers/usb/musb/musb_dsps.c @@ -592,20 +592,8 @@ static int dsps_musb_reset(struct musb *musb) if (glue->sw_babble_enabled) session_restart = dsps_sw_babble_control(musb); - /* - * In case of new silicon version babble condition can be recovered - * without resetting the MUSB. But for older silicon versions, MUSB - * reset is needed - */ - if (session_restart || !glue->sw_babble_enabled) { - dev_info(musb->controller, "Restarting MUSB to recover from Babble\n"); - dsps_writel(musb->ctrl_base, wrp->control, (1 << wrp->reset)); - usleep_range(100, 200); - usb_phy_shutdown(musb->xceiv); - usleep_range(100, 200); - usb_phy_init(musb->xceiv); + else session_restart = 1; - } return session_restart ? 0 : -EPIPE; } > > -- > balbi -- 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