diff mbox

[09/11] usb: musb: break the huge isr musb_stage0_irq() into small functions

Message ID 1523906111-2328-10-git-send-email-b-liu@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bin Liu April 16, 2018, 7:15 p.m. UTC
musb_stage0_irq() is 400+ lines long. Break its interrupt events
handling into each individual functions to make it easy to read.

Signed-off-by: Bin Liu <b-liu@ti.com>
---
 drivers/usb/musb/musb_core.c | 730 +++++++++++++++++++++++--------------------
 1 file changed, 384 insertions(+), 346 deletions(-)

Comments

Dan Carpenter April 18, 2018, 7:25 a.m. UTC | #1
Hi Bin,

I love your patch! Perhaps something to improve:

[auto build test WARNING on balbi-usb/next]
[also build test WARNING on v4.17-rc1 next-20180417]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Bin-Liu/usb-musb-cleanup/20180417-133633
base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next

smatch warnings:
drivers/usb/musb/musb_core.c:797 musb_handle_intr_connect() error: we previously assumed 'musb->hcd' could be null (see line 783)

# https://github.com/0day-ci/linux/commit/92be75b4fb79173759af42e50a81d0020e945c1e
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 92be75b4fb79173759af42e50a81d0020e945c1e
vim +797 drivers/usb/musb/musb_core.c

1c25fda4a Arnaud Mandy              2009-12-28  745  
92be75b4f Bin Liu                   2018-04-16  746  static void musb_handle_intr_connect(struct musb *musb, u8 devctl, u8 int_usb)
92be75b4f Bin Liu                   2018-04-16  747  {
8b125df5b Daniel Mack               2013-04-10  748  	struct usb_hcd *hcd = musb->hcd;
550a7375f Felipe Balbi              2008-07-24  749  
550a7375f Felipe Balbi              2008-07-24  750  	musb->is_active = 1;
550a7375f Felipe Balbi              2008-07-24  751  	musb->ep0_stage = MUSB_EP0_START;
550a7375f Felipe Balbi              2008-07-24  752  
b18d26f6a Sebastian Andrzej Siewior 2012-10-30  753  	musb->intrtxe = musb->epmask;
b18d26f6a Sebastian Andrzej Siewior 2012-10-30  754  	musb_writew(musb->mregs, MUSB_INTRTXE, musb->intrtxe);
af5ec14d4 Sebastian Andrzej Siewior 2012-10-30  755  	musb->intrrxe = musb->epmask & 0xfffe;
af5ec14d4 Sebastian Andrzej Siewior 2012-10-30  756  	musb_writew(musb->mregs, MUSB_INTRRXE, musb->intrrxe);
d709d22ee Ajay Kumar Gupta          2010-07-08  757  	musb_writeb(musb->mregs, MUSB_INTRUSBE, 0xf7);
550a7375f Felipe Balbi              2008-07-24  758  	musb->port1_status &= ~(USB_PORT_STAT_LOW_SPEED
550a7375f Felipe Balbi              2008-07-24  759  				|USB_PORT_STAT_HIGH_SPEED
550a7375f Felipe Balbi              2008-07-24  760  				|USB_PORT_STAT_ENABLE
550a7375f Felipe Balbi              2008-07-24  761  				);
550a7375f Felipe Balbi              2008-07-24  762  	musb->port1_status |= USB_PORT_STAT_CONNECTION
550a7375f Felipe Balbi              2008-07-24  763  				|(USB_PORT_STAT_C_CONNECTION << 16);
550a7375f Felipe Balbi              2008-07-24  764  
550a7375f Felipe Balbi              2008-07-24  765  	/* high vs full speed is just a guess until after reset */
550a7375f Felipe Balbi              2008-07-24  766  	if (devctl & MUSB_DEVCTL_LSDEV)
550a7375f Felipe Balbi              2008-07-24  767  		musb->port1_status |= USB_PORT_STAT_LOW_SPEED;
550a7375f Felipe Balbi              2008-07-24  768  
550a7375f Felipe Balbi              2008-07-24  769  	/* indicate new connection to OTG machine */
e47d92545 Antoine Tenart            2014-10-30  770  	switch (musb->xceiv->otg->state) {
550a7375f Felipe Balbi              2008-07-24  771  	case OTG_STATE_B_PERIPHERAL:
550a7375f Felipe Balbi              2008-07-24  772  		if (int_usb & MUSB_INTR_SUSPEND) {
b99d3659b Bin Liu                   2016-06-30  773  			musb_dbg(musb, "HNP: SUSPEND+CONNECT, now b_host");
550a7375f Felipe Balbi              2008-07-24  774  			int_usb &= ~MUSB_INTR_SUSPEND;
1de00dae8 David Brownell            2009-04-02  775  			goto b_host;
550a7375f Felipe Balbi              2008-07-24  776  		} else
b99d3659b Bin Liu                   2016-06-30  777  			musb_dbg(musb, "CONNECT as b_peripheral???");
550a7375f Felipe Balbi              2008-07-24  778  		break;
550a7375f Felipe Balbi              2008-07-24  779  	case OTG_STATE_B_WAIT_ACON:
b99d3659b Bin Liu                   2016-06-30  780  		musb_dbg(musb, "HNP: CONNECT, now b_host");
1de00dae8 David Brownell            2009-04-02  781  b_host:
e47d92545 Antoine Tenart            2014-10-30  782  		musb->xceiv->otg->state = OTG_STATE_B_HOST;
74c2e9360 Daniel Mack               2013-04-10 @783  		if (musb->hcd)
74c2e9360 Daniel Mack               2013-04-10  784  			musb->hcd->self.is_b_host = 1;
1de00dae8 David Brownell            2009-04-02  785  		del_timer(&musb->otg_timer);
550a7375f Felipe Balbi              2008-07-24  786  		break;
550a7375f Felipe Balbi              2008-07-24  787  	default:
550a7375f Felipe Balbi              2008-07-24  788  		if ((devctl & MUSB_DEVCTL_VBUS)
550a7375f Felipe Balbi              2008-07-24  789  				== (3 << MUSB_DEVCTL_VBUS_SHIFT)) {
e47d92545 Antoine Tenart            2014-10-30  790  			musb->xceiv->otg->state = OTG_STATE_A_HOST;
0b3eba442 Daniel Mack               2013-04-10  791  			if (hcd)
550a7375f Felipe Balbi              2008-07-24  792  				hcd->self.is_b_host = 0;
550a7375f Felipe Balbi              2008-07-24  793  		}
550a7375f Felipe Balbi              2008-07-24  794  		break;
550a7375f Felipe Balbi              2008-07-24  795  	}
1de00dae8 David Brownell            2009-04-02  796  
0b3eba442 Daniel Mack               2013-04-10 @797  	musb_host_poke_root_hub(musb);
1de00dae8 David Brownell            2009-04-02  798  
b99d3659b Bin Liu                   2016-06-30  799  	musb_dbg(musb, "CONNECT (%s) devctl %02x",
e47d92545 Antoine Tenart            2014-10-30  800  			usb_otg_state_string(musb->xceiv->otg->state), devctl);
550a7375f Felipe Balbi              2008-07-24  801  }
550a7375f Felipe Balbi              2008-07-24  802  


---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bin Liu April 18, 2018, 1:33 p.m. UTC | #2
Hi Dan,

I appreciate you scaning  the patches and reporting the issues.

On Wed, Apr 18, 2018 at 10:25:34AM +0300, Dan Carpenter wrote:
> Hi Bin,
> 
> I love your patch! Perhaps something to improve:
> 
> [auto build test WARNING on balbi-usb/next]
> [also build test WARNING on v4.17-rc1 next-20180417]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Bin-Liu/usb-musb-cleanup/20180417-133633
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> 
> smatch warnings:
> drivers/usb/musb/musb_core.c:797 musb_handle_intr_connect() error: we previously assumed 'musb->hcd' could be null (see line 783)

It appears the condition check was introduced back in 2013 in an attempt
to separate host-only and gadget-only configurations for musb [1], but it
seems the work is not complete, because the musb->hcd allocation is
unconditional. So in the current code, musb->hcd won't be NULL. 

But I am now hesitated to remove this 'if(musb->hcd)' check to solve
this smatch warning, because I am still debating on either continue the
work to separate the two configurations or clean up the uncompleted code
left by the attempt to not separate them.

So I want to leave the patch as is for now, is it reasonable?

[1] commit 74c2e93600581("usb: musb: factor out hcd initalization")

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dan Carpenter April 18, 2018, 2:14 p.m. UTC | #3
On Wed, Apr 18, 2018 at 08:33:52AM -0500, Bin Liu wrote:
> Hi Dan,
> 
> I appreciate you scaning  the patches and reporting the issues.
> 

These are kbuild stuff so I basically just forward them.  It's no
effort.

> On Wed, Apr 18, 2018 at 10:25:34AM +0300, Dan Carpenter wrote:
> > Hi Bin,
> > 
> > I love your patch! Perhaps something to improve:
> > 
> > [auto build test WARNING on balbi-usb/next]
> > [also build test WARNING on v4.17-rc1 next-20180417]
> > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> > 
> > url:    https://github.com/0day-ci/linux/commits/Bin-Liu/usb-musb-cleanup/20180417-133633
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> > 
> > smatch warnings:
> > drivers/usb/musb/musb_core.c:797 musb_handle_intr_connect() error: we previously assumed 'musb->hcd' could be null (see line 783)
> 
> It appears the condition check was introduced back in 2013 in an attempt
> to separate host-only and gadget-only configurations for musb [1], but it
> seems the work is not complete, because the musb->hcd allocation is
> unconditional. So in the current code, musb->hcd won't be NULL. 
> 
> But I am now hesitated to remove this 'if(musb->hcd)' check to solve
> this smatch warning, because I am still debating on either continue the
> work to separate the two configurations or clean up the uncompleted code
> left by the attempt to not separate them.
> 
> So I want to leave the patch as is for now, is it reasonable?
> 

Fine by me.  Right now it's sort of hard for Smatch to parse the code
well enough for it to know that ->hcd is always non-NULL but I'm
hopeful that maybe in a couple years that will be possible...

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bin Liu April 18, 2018, 2:22 p.m. UTC | #4
On Wed, Apr 18, 2018 at 05:14:05PM +0300, Dan Carpenter wrote:
> On Wed, Apr 18, 2018 at 08:33:52AM -0500, Bin Liu wrote:
> > Hi Dan,
> > 
> > I appreciate you scaning  the patches and reporting the issues.

The report is valuable!

> > 
> 
> These are kbuild stuff so I basically just forward them.  It's no
> effort.
> 
> > On Wed, Apr 18, 2018 at 10:25:34AM +0300, Dan Carpenter wrote:
> > > Hi Bin,
> > > 
> > > I love your patch! Perhaps something to improve:
> > > 
> > > [auto build test WARNING on balbi-usb/next]
> > > [also build test WARNING on v4.17-rc1 next-20180417]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> > > 
> > > url:    https://github.com/0day-ci/linux/commits/Bin-Liu/usb-musb-cleanup/20180417-133633
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> > > 
> > > smatch warnings:
> > > drivers/usb/musb/musb_core.c:797 musb_handle_intr_connect() error: we previously assumed 'musb->hcd' could be null (see line 783)
> > 
> > It appears the condition check was introduced back in 2013 in an attempt
> > to separate host-only and gadget-only configurations for musb [1], but it
> > seems the work is not complete, because the musb->hcd allocation is
> > unconditional. So in the current code, musb->hcd won't be NULL. 
> > 
> > But I am now hesitated to remove this 'if(musb->hcd)' check to solve
> > this smatch warning, because I am still debating on either continue the
> > work to separate the two configurations or clean up the uncompleted code
> > left by the attempt to not separate them.
> > 
> > So I want to leave the patch as is for now, is it reasonable?
> > 
> 
> Fine by me.  Right now it's sort of hard for Smatch to parse the code
> well enough for it to know that ->hcd is always non-NULL but I'm
> hopeful that maybe in a couple years that will be possible...

To me, the report is good enough :) In this case, it tells the driver
requires some work, which is valid.

Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index ce54f48314e1..a3a716197dc1 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -523,6 +523,383 @@  void musb_hnp_stop(struct musb *musb)
 
 static void musb_recover_from_babble(struct musb *musb);
 
+static void musb_handle_intr_resume(struct musb *musb, u8 devctl)
+{
+	musb_dbg(musb, "RESUME (%s)",
+			usb_otg_state_string(musb->xceiv->otg->state));
+
+	if (devctl & MUSB_DEVCTL_HM) {
+		switch (musb->xceiv->otg->state) {
+		case OTG_STATE_A_SUSPEND:
+			/* remote wakeup? */
+			musb->port1_status |=
+					(USB_PORT_STAT_C_SUSPEND << 16)
+					| MUSB_PORT_STAT_RESUME;
+			musb->rh_timer = jiffies
+				+ msecs_to_jiffies(USB_RESUME_TIMEOUT);
+			musb->xceiv->otg->state = OTG_STATE_A_HOST;
+			musb->is_active = 1;
+			musb_host_resume_root_hub(musb);
+			schedule_delayed_work(&musb->finish_resume_work,
+				msecs_to_jiffies(USB_RESUME_TIMEOUT));
+			break;
+		case OTG_STATE_B_WAIT_ACON:
+			musb->xceiv->otg->state = OTG_STATE_B_PERIPHERAL;
+			musb->is_active = 1;
+			MUSB_DEV_MODE(musb);
+			break;
+		default:
+			WARNING("bogus %s RESUME (%s)\n",
+				"host",
+				usb_otg_state_string(musb->xceiv->otg->state));
+		}
+	} else {
+		switch (musb->xceiv->otg->state) {
+		case OTG_STATE_A_SUSPEND:
+			/* possibly DISCONNECT is upcoming */
+			musb->xceiv->otg->state = OTG_STATE_A_HOST;
+			musb_host_resume_root_hub(musb);
+			break;
+		case OTG_STATE_B_WAIT_ACON:
+		case OTG_STATE_B_PERIPHERAL:
+			/* disconnect while suspended?  we may
+			 * not get a disconnect irq...
+			 */
+			if ((devctl & MUSB_DEVCTL_VBUS)
+					!= (3 << MUSB_DEVCTL_VBUS_SHIFT)
+					) {
+				musb->int_usb |= MUSB_INTR_DISCONNECT;
+				musb->int_usb &= ~MUSB_INTR_SUSPEND;
+				break;
+			}
+			musb_g_resume(musb);
+			break;
+		case OTG_STATE_B_IDLE:
+			musb->int_usb &= ~MUSB_INTR_SUSPEND;
+			break;
+		default:
+			WARNING("bogus %s RESUME (%s)\n",
+				"peripheral",
+				usb_otg_state_string(musb->xceiv->otg->state));
+		}
+	}
+}
+
+/* return IRQ_HANDLED to tell the caller to return immediately */
+static irqreturn_t musb_handle_intr_sessreq(struct musb *musb, u8 devctl)
+{
+	void __iomem *mbase = musb->mregs;
+
+	if ((devctl & MUSB_DEVCTL_VBUS) == MUSB_DEVCTL_VBUS
+			&& (devctl & MUSB_DEVCTL_BDEVICE)) {
+		musb_dbg(musb, "SessReq while on B state");
+		return IRQ_HANDLED;
+	}
+
+	musb_dbg(musb, "SESSION_REQUEST (%s)",
+		usb_otg_state_string(musb->xceiv->otg->state));
+
+	/* IRQ arrives from ID pin sense or (later, if VBUS power
+	 * is removed) SRP.  responses are time critical:
+	 *  - turn on VBUS (with silicon-specific mechanism)
+	 *  - go through A_WAIT_VRISE
+	 *  - ... to A_WAIT_BCON.
+	 * a_wait_vrise_tmout triggers VBUS_ERROR transitions
+	 */
+	musb_writeb(mbase, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);
+	musb->ep0_stage = MUSB_EP0_START;
+	musb->xceiv->otg->state = OTG_STATE_A_IDLE;
+	MUSB_HST_MODE(musb);
+	musb_platform_set_vbus(musb, 1);
+
+	return IRQ_NONE;
+}
+
+static void musb_handle_intr_vbuserr(struct musb *musb, u8 devctl)
+{
+	int	ignore = 0;
+
+	/* During connection as an A-Device, we may see a short
+	 * current spikes causing voltage drop, because of cable
+	 * and peripheral capacitance combined with vbus draw.
+	 * (So: less common with truly self-powered devices, where
+	 * vbus doesn't act like a power supply.)
+	 *
+	 * Such spikes are short; usually less than ~500 usec, max
+	 * of ~2 msec.  That is, they're not sustained overcurrent
+	 * errors, though they're reported using VBUSERROR irqs.
+	 *
+	 * Workarounds:  (a) hardware: use self powered devices.
+	 * (b) software:  ignore non-repeated VBUS errors.
+	 *
+	 * REVISIT:  do delays from lots of DEBUG_KERNEL checks
+	 * make trouble here, keeping VBUS < 4.4V ?
+	 */
+	switch (musb->xceiv->otg->state) {
+	case OTG_STATE_A_HOST:
+		/* recovery is dicey once we've gotten past the
+		 * initial stages of enumeration, but if VBUS
+		 * stayed ok at the other end of the link, and
+		 * another reset is due (at least for high speed,
+		 * to redo the chirp etc), it might work OK...
+		 */
+	case OTG_STATE_A_WAIT_BCON:
+	case OTG_STATE_A_WAIT_VRISE:
+		if (musb->vbuserr_retry) {
+			void __iomem *mbase = musb->mregs;
+
+			musb->vbuserr_retry--;
+			ignore = 1;
+			devctl |= MUSB_DEVCTL_SESSION;
+			musb_writeb(mbase, MUSB_DEVCTL, devctl);
+		} else {
+			musb->port1_status |=
+				  USB_PORT_STAT_OVERCURRENT
+				| (USB_PORT_STAT_C_OVERCURRENT << 16);
+		}
+		break;
+	default:
+		break;
+	}
+
+	dev_printk(ignore ? KERN_DEBUG : KERN_ERR, musb->controller,
+			"VBUS_ERROR in %s (%02x, %s), retry #%d, port1 %08x\n",
+			usb_otg_state_string(musb->xceiv->otg->state),
+			devctl,
+			({ char *s;
+			switch (devctl & MUSB_DEVCTL_VBUS) {
+			case 0 << MUSB_DEVCTL_VBUS_SHIFT:
+				s = "<SessEnd"; break;
+			case 1 << MUSB_DEVCTL_VBUS_SHIFT:
+				s = "<AValid"; break;
+			case 2 << MUSB_DEVCTL_VBUS_SHIFT:
+				s = "<VBusValid"; break;
+			/* case 3 << MUSB_DEVCTL_VBUS_SHIFT: */
+			default:
+				s = "VALID"; break;
+			} s; }),
+			VBUSERR_RETRY_COUNT - musb->vbuserr_retry,
+			musb->port1_status);
+
+	/* go through A_WAIT_VFALL then start a new session */
+	if (!ignore)
+		musb_platform_set_vbus(musb, 0);
+}
+
+static void musb_handle_intr_suspend(struct musb *musb, u8 devctl)
+{
+	musb_dbg(musb, "SUSPEND (%s) devctl %02x",
+		usb_otg_state_string(musb->xceiv->otg->state), devctl);
+
+	switch (musb->xceiv->otg->state) {
+	case OTG_STATE_A_PERIPHERAL:
+		/* We also come here if the cable is removed, since
+		 * this silicon doesn't report ID-no-longer-grounded.
+		 *
+		 * We depend on T(a_wait_bcon) to shut us down, and
+		 * hope users don't do anything dicey during this
+		 * undesired detour through A_WAIT_BCON.
+		 */
+		musb_hnp_stop(musb);
+		musb_host_resume_root_hub(musb);
+		musb_root_disconnect(musb);
+		musb_platform_try_idle(musb, jiffies
+				+ msecs_to_jiffies(musb->a_wait_bcon
+					? : OTG_TIME_A_WAIT_BCON));
+
+		break;
+	case OTG_STATE_B_IDLE:
+		if (!musb->is_active)
+			break;
+		/* fall through */
+	case OTG_STATE_B_PERIPHERAL:
+		musb_g_suspend(musb);
+		musb->is_active = musb->g.b_hnp_enable;
+		if (musb->is_active) {
+			musb->xceiv->otg->state = OTG_STATE_B_WAIT_ACON;
+			musb_dbg(musb, "HNP: Setting timer for b_ase0_brst");
+			mod_timer(&musb->otg_timer, jiffies
+				+ msecs_to_jiffies(
+						OTG_TIME_B_ASE0_BRST));
+		}
+		break;
+	case OTG_STATE_A_WAIT_BCON:
+		if (musb->a_wait_bcon != 0)
+			musb_platform_try_idle(musb, jiffies
+				+ msecs_to_jiffies(musb->a_wait_bcon));
+		break;
+	case OTG_STATE_A_HOST:
+		musb->xceiv->otg->state = OTG_STATE_A_SUSPEND;
+		musb->is_active = musb->hcd->self.b_hnp_enable;
+		break;
+	case OTG_STATE_B_HOST:
+		/* Transition to B_PERIPHERAL, see 6.8.2.6 p 44 */
+		musb_dbg(musb, "REVISIT: SUSPEND as B_HOST");
+		break;
+	default:
+		/* "should not happen" */
+		musb->is_active = 0;
+		break;
+	}
+}
+
+static void musb_handle_intr_connect(struct musb *musb, u8 devctl, u8 int_usb)
+{
+	struct usb_hcd *hcd = musb->hcd;
+
+	musb->is_active = 1;
+	musb->ep0_stage = MUSB_EP0_START;
+
+	musb->intrtxe = musb->epmask;
+	musb_writew(musb->mregs, MUSB_INTRTXE, musb->intrtxe);
+	musb->intrrxe = musb->epmask & 0xfffe;
+	musb_writew(musb->mregs, MUSB_INTRRXE, musb->intrrxe);
+	musb_writeb(musb->mregs, MUSB_INTRUSBE, 0xf7);
+	musb->port1_status &= ~(USB_PORT_STAT_LOW_SPEED
+				|USB_PORT_STAT_HIGH_SPEED
+				|USB_PORT_STAT_ENABLE
+				);
+	musb->port1_status |= USB_PORT_STAT_CONNECTION
+				|(USB_PORT_STAT_C_CONNECTION << 16);
+
+	/* high vs full speed is just a guess until after reset */
+	if (devctl & MUSB_DEVCTL_LSDEV)
+		musb->port1_status |= USB_PORT_STAT_LOW_SPEED;
+
+	/* indicate new connection to OTG machine */
+	switch (musb->xceiv->otg->state) {
+	case OTG_STATE_B_PERIPHERAL:
+		if (int_usb & MUSB_INTR_SUSPEND) {
+			musb_dbg(musb, "HNP: SUSPEND+CONNECT, now b_host");
+			int_usb &= ~MUSB_INTR_SUSPEND;
+			goto b_host;
+		} else
+			musb_dbg(musb, "CONNECT as b_peripheral???");
+		break;
+	case OTG_STATE_B_WAIT_ACON:
+		musb_dbg(musb, "HNP: CONNECT, now b_host");
+b_host:
+		musb->xceiv->otg->state = OTG_STATE_B_HOST;
+		if (musb->hcd)
+			musb->hcd->self.is_b_host = 1;
+		del_timer(&musb->otg_timer);
+		break;
+	default:
+		if ((devctl & MUSB_DEVCTL_VBUS)
+				== (3 << MUSB_DEVCTL_VBUS_SHIFT)) {
+			musb->xceiv->otg->state = OTG_STATE_A_HOST;
+			if (hcd)
+				hcd->self.is_b_host = 0;
+		}
+		break;
+	}
+
+	musb_host_poke_root_hub(musb);
+
+	musb_dbg(musb, "CONNECT (%s) devctl %02x",
+			usb_otg_state_string(musb->xceiv->otg->state), devctl);
+}
+
+static void musb_handle_intr_disconnect(struct musb *musb, u8 devctl)
+{
+	musb_dbg(musb, "DISCONNECT (%s) as %s, devctl %02x",
+			usb_otg_state_string(musb->xceiv->otg->state),
+			MUSB_MODE(musb), devctl);
+
+	switch (musb->xceiv->otg->state) {
+	case OTG_STATE_A_HOST:
+	case OTG_STATE_A_SUSPEND:
+		musb_host_resume_root_hub(musb);
+		musb_root_disconnect(musb);
+		if (musb->a_wait_bcon != 0)
+			musb_platform_try_idle(musb, jiffies
+				+ msecs_to_jiffies(musb->a_wait_bcon));
+		break;
+	case OTG_STATE_B_HOST:
+		/* REVISIT this behaves for "real disconnect"
+		 * cases; make sure the other transitions from
+		 * from B_HOST act right too.  The B_HOST code
+		 * in hnp_stop() is currently not used...
+		 */
+		musb_root_disconnect(musb);
+		if (musb->hcd)
+			musb->hcd->self.is_b_host = 0;
+		musb->xceiv->otg->state = OTG_STATE_B_PERIPHERAL;
+		MUSB_DEV_MODE(musb);
+		musb_g_disconnect(musb);
+		break;
+	case OTG_STATE_A_PERIPHERAL:
+		musb_hnp_stop(musb);
+		musb_root_disconnect(musb);
+		/* FALLTHROUGH */
+	case OTG_STATE_B_WAIT_ACON:
+		/* FALLTHROUGH */
+	case OTG_STATE_B_PERIPHERAL:
+	case OTG_STATE_B_IDLE:
+		musb_g_disconnect(musb);
+		break;
+	default:
+		WARNING("unhandled DISCONNECT transition (%s)\n",
+			usb_otg_state_string(musb->xceiv->otg->state));
+		break;
+	}
+}
+
+/*
+ * mentor saves a bit: bus reset and babble share the same irq.
+ * only host sees babble; only peripheral sees bus reset.
+ */
+static void musb_handle_intr_reset(struct musb *musb)
+{
+	if (is_host_active(musb)) {
+		/*
+		 * When BABBLE happens what we can depends on which
+		 * platform MUSB is running, because some platforms
+		 * implemented proprietary means for 'recovering' from
+		 * Babble conditions. One such platform is AM335x. In
+		 * most cases, however, the only thing we can do is
+		 * drop the session.
+		 */
+		dev_err(musb->controller, "Babble\n");
+		musb_recover_from_babble(musb);
+	} else {
+		musb_dbg(musb, "BUS RESET as %s",
+			usb_otg_state_string(musb->xceiv->otg->state));
+		switch (musb->xceiv->otg->state) {
+		case OTG_STATE_A_SUSPEND:
+			musb_g_reset(musb);
+			/* FALLTHROUGH */
+		case OTG_STATE_A_WAIT_BCON:	/* OPT TD.4.7-900ms */
+			/* never use invalid T(a_wait_bcon) */
+			musb_dbg(musb, "HNP: in %s, %d msec timeout",
+				usb_otg_state_string(musb->xceiv->otg->state),
+				TA_WAIT_BCON(musb));
+			mod_timer(&musb->otg_timer, jiffies
+				+ msecs_to_jiffies(TA_WAIT_BCON(musb)));
+			break;
+		case OTG_STATE_A_PERIPHERAL:
+			del_timer(&musb->otg_timer);
+			musb_g_reset(musb);
+			break;
+		case OTG_STATE_B_WAIT_ACON:
+			musb_dbg(musb, "HNP: RESET (%s), to b_peripheral",
+				usb_otg_state_string(musb->xceiv->otg->state));
+			musb->xceiv->otg->state = OTG_STATE_B_PERIPHERAL;
+			musb_g_reset(musb);
+			break;
+		case OTG_STATE_B_IDLE:
+			musb->xceiv->otg->state = OTG_STATE_B_PERIPHERAL;
+			/* FALLTHROUGH */
+		case OTG_STATE_B_PERIPHERAL:
+			musb_g_reset(musb);
+			break;
+		default:
+			musb_dbg(musb, "Unhandled BUS RESET as %s",
+				usb_otg_state_string(musb->xceiv->otg->state));
+		}
+	}
+}
+
 /*
  * Interrupt Service Routine to record USB "global" interrupts.
  * Since these do not happen often and signify things of
@@ -547,379 +924,40 @@  static irqreturn_t musb_stage0_irq(struct musb *musb, u8 int_usb,
 	 * spurious RESUME irqs happen too, paired with SUSPEND.
 	 */
 	if (int_usb & MUSB_INTR_RESUME) {
+		musb_handle_intr_resume(musb, devctl);
 		handled = IRQ_HANDLED;
-		musb_dbg(musb, "RESUME (%s)",
-				usb_otg_state_string(musb->xceiv->otg->state));
-
-		if (devctl & MUSB_DEVCTL_HM) {
-			switch (musb->xceiv->otg->state) {
-			case OTG_STATE_A_SUSPEND:
-				/* remote wakeup? */
-				musb->port1_status |=
-						(USB_PORT_STAT_C_SUSPEND << 16)
-						| MUSB_PORT_STAT_RESUME;
-				musb->rh_timer = jiffies
-					+ msecs_to_jiffies(USB_RESUME_TIMEOUT);
-				musb->xceiv->otg->state = OTG_STATE_A_HOST;
-				musb->is_active = 1;
-				musb_host_resume_root_hub(musb);
-				schedule_delayed_work(&musb->finish_resume_work,
-					msecs_to_jiffies(USB_RESUME_TIMEOUT));
-				break;
-			case OTG_STATE_B_WAIT_ACON:
-				musb->xceiv->otg->state = OTG_STATE_B_PERIPHERAL;
-				musb->is_active = 1;
-				MUSB_DEV_MODE(musb);
-				break;
-			default:
-				WARNING("bogus %s RESUME (%s)\n",
-					"host",
-					usb_otg_state_string(musb->xceiv->otg->state));
-			}
-		} else {
-			switch (musb->xceiv->otg->state) {
-			case OTG_STATE_A_SUSPEND:
-				/* possibly DISCONNECT is upcoming */
-				musb->xceiv->otg->state = OTG_STATE_A_HOST;
-				musb_host_resume_root_hub(musb);
-				break;
-			case OTG_STATE_B_WAIT_ACON:
-			case OTG_STATE_B_PERIPHERAL:
-				/* disconnect while suspended?  we may
-				 * not get a disconnect irq...
-				 */
-				if ((devctl & MUSB_DEVCTL_VBUS)
-						!= (3 << MUSB_DEVCTL_VBUS_SHIFT)
-						) {
-					musb->int_usb |= MUSB_INTR_DISCONNECT;
-					musb->int_usb &= ~MUSB_INTR_SUSPEND;
-					break;
-				}
-				musb_g_resume(musb);
-				break;
-			case OTG_STATE_B_IDLE:
-				musb->int_usb &= ~MUSB_INTR_SUSPEND;
-				break;
-			default:
-				WARNING("bogus %s RESUME (%s)\n",
-					"peripheral",
-					usb_otg_state_string(musb->xceiv->otg->state));
-			}
-		}
 	}
 
 	/* see manual for the order of the tests */
 	if (int_usb & MUSB_INTR_SESSREQ) {
-		void __iomem *mbase = musb->mregs;
-
-		if ((devctl & MUSB_DEVCTL_VBUS) == MUSB_DEVCTL_VBUS
-				&& (devctl & MUSB_DEVCTL_BDEVICE)) {
-			musb_dbg(musb, "SessReq while on B state");
+		if (musb_handle_intr_sessreq(musb, devctl))
 			return IRQ_HANDLED;
-		}
-
-		musb_dbg(musb, "SESSION_REQUEST (%s)",
-			usb_otg_state_string(musb->xceiv->otg->state));
-
-		/* IRQ arrives from ID pin sense or (later, if VBUS power
-		 * is removed) SRP.  responses are time critical:
-		 *  - turn on VBUS (with silicon-specific mechanism)
-		 *  - go through A_WAIT_VRISE
-		 *  - ... to A_WAIT_BCON.
-		 * a_wait_vrise_tmout triggers VBUS_ERROR transitions
-		 */
-		musb_writeb(mbase, MUSB_DEVCTL, MUSB_DEVCTL_SESSION);
-		musb->ep0_stage = MUSB_EP0_START;
-		musb->xceiv->otg->state = OTG_STATE_A_IDLE;
-		MUSB_HST_MODE(musb);
-		musb_platform_set_vbus(musb, 1);
-
 		handled = IRQ_HANDLED;
 	}
 
 	if (int_usb & MUSB_INTR_VBUSERROR) {
-		int	ignore = 0;
-
-		/* During connection as an A-Device, we may see a short
-		 * current spikes causing voltage drop, because of cable
-		 * and peripheral capacitance combined with vbus draw.
-		 * (So: less common with truly self-powered devices, where
-		 * vbus doesn't act like a power supply.)
-		 *
-		 * Such spikes are short; usually less than ~500 usec, max
-		 * of ~2 msec.  That is, they're not sustained overcurrent
-		 * errors, though they're reported using VBUSERROR irqs.
-		 *
-		 * Workarounds:  (a) hardware: use self powered devices.
-		 * (b) software:  ignore non-repeated VBUS errors.
-		 *
-		 * REVISIT:  do delays from lots of DEBUG_KERNEL checks
-		 * make trouble here, keeping VBUS < 4.4V ?
-		 */
-		switch (musb->xceiv->otg->state) {
-		case OTG_STATE_A_HOST:
-			/* recovery is dicey once we've gotten past the
-			 * initial stages of enumeration, but if VBUS
-			 * stayed ok at the other end of the link, and
-			 * another reset is due (at least for high speed,
-			 * to redo the chirp etc), it might work OK...
-			 */
-		case OTG_STATE_A_WAIT_BCON:
-		case OTG_STATE_A_WAIT_VRISE:
-			if (musb->vbuserr_retry) {
-				void __iomem *mbase = musb->mregs;
-
-				musb->vbuserr_retry--;
-				ignore = 1;
-				devctl |= MUSB_DEVCTL_SESSION;
-				musb_writeb(mbase, MUSB_DEVCTL, devctl);
-			} else {
-				musb->port1_status |=
-					  USB_PORT_STAT_OVERCURRENT
-					| (USB_PORT_STAT_C_OVERCURRENT << 16);
-			}
-			break;
-		default:
-			break;
-		}
-
-		dev_printk(ignore ? KERN_DEBUG : KERN_ERR, musb->controller,
-				"VBUS_ERROR in %s (%02x, %s), retry #%d, port1 %08x\n",
-				usb_otg_state_string(musb->xceiv->otg->state),
-				devctl,
-				({ char *s;
-				switch (devctl & MUSB_DEVCTL_VBUS) {
-				case 0 << MUSB_DEVCTL_VBUS_SHIFT:
-					s = "<SessEnd"; break;
-				case 1 << MUSB_DEVCTL_VBUS_SHIFT:
-					s = "<AValid"; break;
-				case 2 << MUSB_DEVCTL_VBUS_SHIFT:
-					s = "<VBusValid"; break;
-				/* case 3 << MUSB_DEVCTL_VBUS_SHIFT: */
-				default:
-					s = "VALID"; break;
-				} s; }),
-				VBUSERR_RETRY_COUNT - musb->vbuserr_retry,
-				musb->port1_status);
-
-		/* go through A_WAIT_VFALL then start a new session */
-		if (!ignore)
-			musb_platform_set_vbus(musb, 0);
+		musb_handle_intr_vbuserr(musb, devctl);
 		handled = IRQ_HANDLED;
 	}
 
 	if (int_usb & MUSB_INTR_SUSPEND) {
-		musb_dbg(musb, "SUSPEND (%s) devctl %02x",
-			usb_otg_state_string(musb->xceiv->otg->state), devctl);
+		musb_handle_intr_suspend(musb, devctl);
 		handled = IRQ_HANDLED;
-
-		switch (musb->xceiv->otg->state) {
-		case OTG_STATE_A_PERIPHERAL:
-			/* We also come here if the cable is removed, since
-			 * this silicon doesn't report ID-no-longer-grounded.
-			 *
-			 * We depend on T(a_wait_bcon) to shut us down, and
-			 * hope users don't do anything dicey during this
-			 * undesired detour through A_WAIT_BCON.
-			 */
-			musb_hnp_stop(musb);
-			musb_host_resume_root_hub(musb);
-			musb_root_disconnect(musb);
-			musb_platform_try_idle(musb, jiffies
-					+ msecs_to_jiffies(musb->a_wait_bcon
-						? : OTG_TIME_A_WAIT_BCON));
-
-			break;
-		case OTG_STATE_B_IDLE:
-			if (!musb->is_active)
-				break;
-			/* fall through */
-		case OTG_STATE_B_PERIPHERAL:
-			musb_g_suspend(musb);
-			musb->is_active = musb->g.b_hnp_enable;
-			if (musb->is_active) {
-				musb->xceiv->otg->state = OTG_STATE_B_WAIT_ACON;
-				musb_dbg(musb, "HNP: Setting timer for b_ase0_brst");
-				mod_timer(&musb->otg_timer, jiffies
-					+ msecs_to_jiffies(
-							OTG_TIME_B_ASE0_BRST));
-			}
-			break;
-		case OTG_STATE_A_WAIT_BCON:
-			if (musb->a_wait_bcon != 0)
-				musb_platform_try_idle(musb, jiffies
-					+ msecs_to_jiffies(musb->a_wait_bcon));
-			break;
-		case OTG_STATE_A_HOST:
-			musb->xceiv->otg->state = OTG_STATE_A_SUSPEND;
-			musb->is_active = musb->hcd->self.b_hnp_enable;
-			break;
-		case OTG_STATE_B_HOST:
-			/* Transition to B_PERIPHERAL, see 6.8.2.6 p 44 */
-			musb_dbg(musb, "REVISIT: SUSPEND as B_HOST");
-			break;
-		default:
-			/* "should not happen" */
-			musb->is_active = 0;
-			break;
-		}
 	}
 
 	if (int_usb & MUSB_INTR_CONNECT) {
-		struct usb_hcd *hcd = musb->hcd;
-
+		musb_handle_intr_connect(musb, devctl, int_usb);
 		handled = IRQ_HANDLED;
-		musb->is_active = 1;
-
-		musb->ep0_stage = MUSB_EP0_START;
-
-		musb->intrtxe = musb->epmask;
-		musb_writew(musb->mregs, MUSB_INTRTXE, musb->intrtxe);
-		musb->intrrxe = musb->epmask & 0xfffe;
-		musb_writew(musb->mregs, MUSB_INTRRXE, musb->intrrxe);
-		musb_writeb(musb->mregs, MUSB_INTRUSBE, 0xf7);
-		musb->port1_status &= ~(USB_PORT_STAT_LOW_SPEED
-					|USB_PORT_STAT_HIGH_SPEED
-					|USB_PORT_STAT_ENABLE
-					);
-		musb->port1_status |= USB_PORT_STAT_CONNECTION
-					|(USB_PORT_STAT_C_CONNECTION << 16);
-
-		/* high vs full speed is just a guess until after reset */
-		if (devctl & MUSB_DEVCTL_LSDEV)
-			musb->port1_status |= USB_PORT_STAT_LOW_SPEED;
-
-		/* indicate new connection to OTG machine */
-		switch (musb->xceiv->otg->state) {
-		case OTG_STATE_B_PERIPHERAL:
-			if (int_usb & MUSB_INTR_SUSPEND) {
-				musb_dbg(musb, "HNP: SUSPEND+CONNECT, now b_host");
-				int_usb &= ~MUSB_INTR_SUSPEND;
-				goto b_host;
-			} else
-				musb_dbg(musb, "CONNECT as b_peripheral???");
-			break;
-		case OTG_STATE_B_WAIT_ACON:
-			musb_dbg(musb, "HNP: CONNECT, now b_host");
-b_host:
-			musb->xceiv->otg->state = OTG_STATE_B_HOST;
-			if (musb->hcd)
-				musb->hcd->self.is_b_host = 1;
-			del_timer(&musb->otg_timer);
-			break;
-		default:
-			if ((devctl & MUSB_DEVCTL_VBUS)
-					== (3 << MUSB_DEVCTL_VBUS_SHIFT)) {
-				musb->xceiv->otg->state = OTG_STATE_A_HOST;
-				if (hcd)
-					hcd->self.is_b_host = 0;
-			}
-			break;
-		}
-
-		musb_host_poke_root_hub(musb);
-
-		musb_dbg(musb, "CONNECT (%s) devctl %02x",
-				usb_otg_state_string(musb->xceiv->otg->state), devctl);
 	}
 
 	if (int_usb & MUSB_INTR_DISCONNECT) {
-		musb_dbg(musb, "DISCONNECT (%s) as %s, devctl %02x",
-				usb_otg_state_string(musb->xceiv->otg->state),
-				MUSB_MODE(musb), devctl);
+		musb_handle_intr_disconnect(musb, devctl);
 		handled = IRQ_HANDLED;
-
-		switch (musb->xceiv->otg->state) {
-		case OTG_STATE_A_HOST:
-		case OTG_STATE_A_SUSPEND:
-			musb_host_resume_root_hub(musb);
-			musb_root_disconnect(musb);
-			if (musb->a_wait_bcon != 0)
-				musb_platform_try_idle(musb, jiffies
-					+ msecs_to_jiffies(musb->a_wait_bcon));
-			break;
-		case OTG_STATE_B_HOST:
-			/* REVISIT this behaves for "real disconnect"
-			 * cases; make sure the other transitions from
-			 * from B_HOST act right too.  The B_HOST code
-			 * in hnp_stop() is currently not used...
-			 */
-			musb_root_disconnect(musb);
-			if (musb->hcd)
-				musb->hcd->self.is_b_host = 0;
-			musb->xceiv->otg->state = OTG_STATE_B_PERIPHERAL;
-			MUSB_DEV_MODE(musb);
-			musb_g_disconnect(musb);
-			break;
-		case OTG_STATE_A_PERIPHERAL:
-			musb_hnp_stop(musb);
-			musb_root_disconnect(musb);
-			/* FALLTHROUGH */
-		case OTG_STATE_B_WAIT_ACON:
-			/* FALLTHROUGH */
-		case OTG_STATE_B_PERIPHERAL:
-		case OTG_STATE_B_IDLE:
-			musb_g_disconnect(musb);
-			break;
-		default:
-			WARNING("unhandled DISCONNECT transition (%s)\n",
-				usb_otg_state_string(musb->xceiv->otg->state));
-			break;
-		}
 	}
 
-	/* mentor saves a bit: bus reset and babble share the same irq.
-	 * only host sees babble; only peripheral sees bus reset.
-	 */
 	if (int_usb & MUSB_INTR_RESET) {
+		musb_handle_intr_reset(musb);
 		handled = IRQ_HANDLED;
-		if (is_host_active(musb)) {
-			/*
-			 * When BABBLE happens what we can depends on which
-			 * platform MUSB is running, because some platforms
-			 * implemented proprietary means for 'recovering' from
-			 * Babble conditions. One such platform is AM335x. In
-			 * most cases, however, the only thing we can do is
-			 * drop the session.
-			 */
-			dev_err(musb->controller, "Babble\n");
-			musb_recover_from_babble(musb);
-		} else {
-			musb_dbg(musb, "BUS RESET as %s",
-				usb_otg_state_string(musb->xceiv->otg->state));
-			switch (musb->xceiv->otg->state) {
-			case OTG_STATE_A_SUSPEND:
-				musb_g_reset(musb);
-				/* FALLTHROUGH */
-			case OTG_STATE_A_WAIT_BCON:	/* OPT TD.4.7-900ms */
-				/* never use invalid T(a_wait_bcon) */
-				musb_dbg(musb, "HNP: in %s, %d msec timeout",
-					usb_otg_state_string(musb->xceiv->otg->state),
-					TA_WAIT_BCON(musb));
-				mod_timer(&musb->otg_timer, jiffies
-					+ msecs_to_jiffies(TA_WAIT_BCON(musb)));
-				break;
-			case OTG_STATE_A_PERIPHERAL:
-				del_timer(&musb->otg_timer);
-				musb_g_reset(musb);
-				break;
-			case OTG_STATE_B_WAIT_ACON:
-				musb_dbg(musb, "HNP: RESET (%s), to b_peripheral",
-					usb_otg_state_string(musb->xceiv->otg->state));
-				musb->xceiv->otg->state = OTG_STATE_B_PERIPHERAL;
-				musb_g_reset(musb);
-				break;
-			case OTG_STATE_B_IDLE:
-				musb->xceiv->otg->state = OTG_STATE_B_PERIPHERAL;
-				/* FALLTHROUGH */
-			case OTG_STATE_B_PERIPHERAL:
-				musb_g_reset(musb);
-				break;
-			default:
-				musb_dbg(musb, "Unhandled BUS RESET as %s",
-					usb_otg_state_string(musb->xceiv->otg->state));
-			}
-		}
 	}
 
 #if 0