pending patch

pending patch

[Felipe Balbi]

Hi Tony and Dave,

there's this one pending patch [1]. Do you guys have any comments on that
one?

Author: Felipe Balbi <felipe.balbi@nokia.com>
Date:   Thu Apr 17 17:34:20 2008 +0300

    USB: MUSB: Don't ignore disconnect on suspend
    
    As soon as a usb device is disconnect we should
    fall into a_wait_bcon state, ignoring disconnect
    irq will prevent this behaviour.
    
    Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index c5816a2..019898a 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -659,7 +659,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
                        switch (musb->xceiv.state) {
 #ifdef CONFIG_USB_OTG
                        case OTG_STATE_A_SUSPEND:
-                               musb->ignore_disconnect = 1;
+                               musb->ignore_disconnect = 0;
                                musb_g_reset(musb);
                                /* FALLTHROUGH */
                        case OTG_STATE_A_WAIT_BCON:     /* OPT TD.4.7-900ms */

[1] http://marc.info/?l=linux-omap&m=120844324526642&w=2
-- 
	- Balbi

Re: pending patch

[David Brownell]

On Friday 25 April 2008, Felipe Balbi wrote:
> Hi Tony and Dave,
> 
> there's this one pending patch [1]. Do you guys have any comments on that
> one?
> 
> Author: Felipe Balbi <felipe.balbi@nokia.com>
> Date:   Thu Apr 17 17:34:20 2008 +0300
> 
>     USB: MUSB: Don't ignore disconnect on suspend
>     
>     As soon as a usb device is disconnect we should
>     fall into a_wait_bcon state, ignoring disconnect
>     irq will prevent this behaviour.

If it passes the OTG tests, fine.  I don't recall how the
HNP handoff's disconnect signaling is handled ... presumably
it's different from some "real" disconnect.



>     Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
> 
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index c5816a2..019898a 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -659,7 +659,7 @@ static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
>                         switch (musb->xceiv.state) {
>  #ifdef CONFIG_USB_OTG
>                         case OTG_STATE_A_SUSPEND:
> -                               musb->ignore_disconnect = 1;
> +                               musb->ignore_disconnect = 0;
>                                 musb_g_reset(musb);
>                                 /* FALLTHROUGH */
>                         case OTG_STATE_A_WAIT_BCON:     /* OPT TD.4.7-900ms */
> 
> [1] http://marc.info/?l=linux-omap&m=120844324526642&w=2
> -- 
> 	- Balbi
> 

Re: pending patch

[Felipe Balbi]

On Fri, Apr 25, 2008 at 02:35:21AM -0700, David Brownell wrote:
> On Friday 25 April 2008, Felipe Balbi wrote:
> > Hi Tony and Dave,
> > 
> > there's this one pending patch [1]. Do you guys have any comments on that
> > one?
> > 
> > Author: Felipe Balbi <felipe.balbi@nokia.com>
> > Date:   Thu Apr 17 17:34:20 2008 +0300
> > 
> >     USB: MUSB: Don't ignore disconnect on suspend
> >     
> >     As soon as a usb device is disconnect we should
> >     fall into a_wait_bcon state, ignoring disconnect
> >     irq will prevent this behaviour.
> 
> If it passes the OTG tests, fine.  I don't recall how the
> HNP handoff's disconnect signaling is handled ... presumably
> it's different from some "real" disconnect.

No opt available for now...
The problem is that when you ignore disconnect on suspend you only ack
the disconnect interrupt after attaching other device. It won't get us
too much trouble besides the bit will still set in int_src, but still
musb should ack disconnect as soon as it happens, shouldn't it? And
right after that start the timer for switching from a_wait_bcon to
a_wait_vfall and after that a_idle.

But as soon as I have and opt available, I'll try to run this test and
report you if I can see any issues with this patch applied.

For now, let's keep it as pending so.

-- 
	- Balbi

Re: pending patch

[Tony Lindgren]

* Felipe Balbi <felipe.balbi@nokia.com> [080425 03:19]:
> On Fri, Apr 25, 2008 at 02:35:21AM -0700, David Brownell wrote:
> > On Friday 25 April 2008, Felipe Balbi wrote:
> > > Hi Tony and Dave,
> > > 
> > > there's this one pending patch [1]. Do you guys have any comments on that
> > > one?
> > > 
> > > Author: Felipe Balbi <felipe.balbi@nokia.com>
> > > Date:   Thu Apr 17 17:34:20 2008 +0300
> > > 
> > >     USB: MUSB: Don't ignore disconnect on suspend
> > >     
> > >     As soon as a usb device is disconnect we should
> > >     fall into a_wait_bcon state, ignoring disconnect
> > >     irq will prevent this behaviour.
> > 
> > If it passes the OTG tests, fine.  I don't recall how the
> > HNP handoff's disconnect signaling is handled ... presumably
> > it's different from some "real" disconnect.
> 
> No opt available for now...
> The problem is that when you ignore disconnect on suspend you only ack
> the disconnect interrupt after attaching other device. It won't get us
> too much trouble besides the bit will still set in int_src, but still
> musb should ack disconnect as soon as it happens, shouldn't it? And
> right after that start the timer for switching from a_wait_bcon to
> a_wait_vfall and after that a_idle.
> 
> But as soon as I have and opt available, I'll try to run this test and
> report you if I can see any issues with this patch applied.
> 
> For now, let's keep it as pending so.

Yeah. I have not applied any musb/tusb patches because of being afraid
of the HS OTG OPT tests breaking.. Still have them all in my l-o mailbox
though. So let's plan on applying things after somebody (Felipe?) gets
to run HS OTG OPT on them for musb and tusb.

Of course this discussion should get moved to linux-usb-devel..

Tony

Re: pending patch

[Felipe Balbi]

On Fri, Apr 25, 2008 at 09:36:44AM -0700, Tony Lindgren wrote:
> * Felipe Balbi <felipe.balbi@nokia.com> [080425 03:19]:
> > On Fri, Apr 25, 2008 at 02:35:21AM -0700, David Brownell wrote:
> > > On Friday 25 April 2008, Felipe Balbi wrote:
> > > > Hi Tony and Dave,
> > > > 
> > > > there's this one pending patch [1]. Do you guys have any comments on that
> > > > one?
> > > > 
> > > > Author: Felipe Balbi <felipe.balbi@nokia.com>
> > > > Date:   Thu Apr 17 17:34:20 2008 +0300
> > > > 
> > > >     USB: MUSB: Don't ignore disconnect on suspend
> > > >     
> > > >     As soon as a usb device is disconnect we should
> > > >     fall into a_wait_bcon state, ignoring disconnect
> > > >     irq will prevent this behaviour.
> > > 
> > > If it passes the OTG tests, fine.  I don't recall how the
> > > HNP handoff's disconnect signaling is handled ... presumably
> > > it's different from some "real" disconnect.
> > 
> > No opt available for now...
> > The problem is that when you ignore disconnect on suspend you only ack
> > the disconnect interrupt after attaching other device. It won't get us
> > too much trouble besides the bit will still set in int_src, but still
> > musb should ack disconnect as soon as it happens, shouldn't it? And
> > right after that start the timer for switching from a_wait_bcon to
> > a_wait_vfall and after that a_idle.
> > 
> > But as soon as I have and opt available, I'll try to run this test and
> > report you if I can see any issues with this patch applied.
> > 
> > For now, let's keep it as pending so.
> 
> Yeah. I have not applied any musb/tusb patches because of being afraid
> of the HS OTG OPT tests breaking.. Still have them all in my l-o mailbox
> though. So let's plan on applying things after somebody (Felipe?) gets
> to run HS OTG OPT on them for musb and tusb.

I think current driver won't pass opt at all. First of all we have a bug
in queued control transfers that gives us an oops. Quite easy to
reproduce: testusb -t 10 (just put a g_zero on the other side).

According to Dave we have two patches on ehci to help with that. I'm
looking for them in the archives without much luck but once I find them
it should be easy to make any other necessary changes to musb driver
(if any). Anyways, I get an oops in drivers/usb/hcd.c:unmap_urb_for_dma,
right in the test if (is_root_hub(urb->dev), tracking back to musb, we
would fall in __musb_giveback_urb()...

Also, we don't have the messaging support (but that's piece of cake).

> Of course this discussion should get moved to linux-usb-devel..

Yeah, i'm waiting to get all opt and testusb stuff ready so I can
prepare patches on top of mailine and send to linux-usb-devel. It's
about time.

-- 
Best Regards,

Felipe Balbi
me@felipebalbi.com
http://blog.felipebalbi.com

Re: pending patch

[Felipe Balbi]

On Fri, Apr 25, 2008 at 09:45:43PM +0300, ext Felipe Balbi wrote:
> On Fri, Apr 25, 2008 at 09:36:44AM -0700, Tony Lindgren wrote:
> > * Felipe Balbi <felipe.balbi@nokia.com> [080425 03:19]:
> > > On Fri, Apr 25, 2008 at 02:35:21AM -0700, David Brownell wrote:
> > > > On Friday 25 April 2008, Felipe Balbi wrote:
> > > > > Hi Tony and Dave,
> > > > > 
> > > > > there's this one pending patch [1]. Do you guys have any comments on that
> > > > > one?
> > > > > 
> > > > > Author: Felipe Balbi <felipe.balbi@nokia.com>
> > > > > Date:   Thu Apr 17 17:34:20 2008 +0300
> > > > > 
> > > > >     USB: MUSB: Don't ignore disconnect on suspend
> > > > >     
> > > > >     As soon as a usb device is disconnect we should
> > > > >     fall into a_wait_bcon state, ignoring disconnect
> > > > >     irq will prevent this behaviour.
> > > > 
> > > > If it passes the OTG tests, fine.  I don't recall how the
> > > > HNP handoff's disconnect signaling is handled ... presumably
> > > > it's different from some "real" disconnect.
> > > 
> > > No opt available for now...
> > > The problem is that when you ignore disconnect on suspend you only ack
> > > the disconnect interrupt after attaching other device. It won't get us
> > > too much trouble besides the bit will still set in int_src, but still
> > > musb should ack disconnect as soon as it happens, shouldn't it? And
> > > right after that start the timer for switching from a_wait_bcon to
> > > a_wait_vfall and after that a_idle.
> > > 
> > > But as soon as I have and opt available, I'll try to run this test and
> > > report you if I can see any issues with this patch applied.
> > > 
> > > For now, let's keep it as pending so.
> > 
> > Yeah. I have not applied any musb/tusb patches because of being afraid
> > of the HS OTG OPT tests breaking.. Still have them all in my l-o mailbox
> > though. So let's plan on applying things after somebody (Felipe?) gets
> > to run HS OTG OPT on them for musb and tusb.

Ok, this patch doesn't break any opt tests. Safe to apply. Tested
yesterday.

-- 
	- Balbi

Re: pending patch

[Tony Lindgren]

* Felipe Balbi <felipe.balbi@nokia.com> [080509 02:20]:
> On Fri, Apr 25, 2008 at 09:45:43PM +0300, ext Felipe Balbi wrote:
> > On Fri, Apr 25, 2008 at 09:36:44AM -0700, Tony Lindgren wrote:
> > > * Felipe Balbi <felipe.balbi@nokia.com> [080425 03:19]:
> > > > On Fri, Apr 25, 2008 at 02:35:21AM -0700, David Brownell wrote:
> > > > > On Friday 25 April 2008, Felipe Balbi wrote:
> > > > > > Hi Tony and Dave,
> > > > > > 
> > > > > > there's this one pending patch [1]. Do you guys have any comments on that
> > > > > > one?
> > > > > > 
> > > > > > Author: Felipe Balbi <felipe.balbi@nokia.com>
> > > > > > Date:   Thu Apr 17 17:34:20 2008 +0300
> > > > > > 
> > > > > >     USB: MUSB: Don't ignore disconnect on suspend
> > > > > >     
> > > > > >     As soon as a usb device is disconnect we should
> > > > > >     fall into a_wait_bcon state, ignoring disconnect
> > > > > >     irq will prevent this behaviour.
> > > > > 
> > > > > If it passes the OTG tests, fine.  I don't recall how the
> > > > > HNP handoff's disconnect signaling is handled ... presumably
> > > > > it's different from some "real" disconnect.
> > > > 
> > > > No opt available for now...
> > > > The problem is that when you ignore disconnect on suspend you only ack
> > > > the disconnect interrupt after attaching other device. It won't get us
> > > > too much trouble besides the bit will still set in int_src, but still
> > > > musb should ack disconnect as soon as it happens, shouldn't it? And
> > > > right after that start the timer for switching from a_wait_bcon to
> > > > a_wait_vfall and after that a_idle.
> > > > 
> > > > But as soon as I have and opt available, I'll try to run this test and
> > > > report you if I can see any issues with this patch applied.
> > > > 
> > > > For now, let's keep it as pending so.
> > > 
> > > Yeah. I have not applied any musb/tusb patches because of being afraid
> > > of the HS OTG OPT tests breaking.. Still have them all in my l-o mailbox
> > > though. So let's plan on applying things after somebody (Felipe?) gets
> > > to run HS OTG OPT on them for musb and tusb.
> 
> Ok, this patch doesn't break any opt tests. Safe to apply. Tested
> yesterday.

Pushing.

Tony