On Thu, Feb 26, 2015 at 2:19 PM, Felipe Balbi <balbi@xxxxxx> 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 <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()
>
> 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 <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..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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
