Hi,
On Thu, Feb 26, 2015 at 02:11:15PM -0600, Felipe Balbi wrote:
> On Thu, Feb 26, 2015 at 02:04:37PM -0600, Bin Liu wrote:
> > On Thu, Feb 26, 2015 at 1:59 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> > > On Thu, Feb 26, 2015 at 01:49:51PM -0600, Bin Liu wrote:
> > >> On Thu, Feb 26, 2015 at 1:48 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> > >> > On Thu, Feb 26, 2015 at 01:30:21PM -0600, Bin Liu wrote:
> > >> >> Felipe,
> > >> >>
> > >> >> On Thu, Feb 26, 2015 at 12:27 PM, Felipe Balbi <balbi@xxxxxx> 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 <balbi@xxxxxx>
> > >> >> >
> > >> >> > 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 <balbi@xxxxxx>
> 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 <balbi@xxxxxx>
>
> 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 <balbi@xxxxxx>
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 <balbi@xxxxxx>
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()
--
balbi
Attachment:
signature.asc
Description: Digital signature
