Re: high cpu load on omap3 using musb

Felipe Balbi <[email protected]> Mon, 21 Jul 2014 10:40:52 -0500
Newsgroups	gmane.linux.ports.arm.general
Message-ID	<20140721154052.GJ6852__21964.0095405795$1405957600$gmane$org@saruman.home>
On Mon, Jul 21, 2014 at 05:28:58PM +0200, Laurent Pinchart wrote:
> Hi Adam,
> 
> On Wednesday 29 January 2014 08:44:57 Adam Wozniak wrote:
> > With a USB 2.0 webcam attached to the OTG port on an OMAP3 (applies to
> > overo gumstix, beagleboard, probably others) we see a high CPU load in a
> > kworker thread.
> > 
> > Between 2.6.33 and 2.6.34 musb_core.c changed.
> > 
> > IRQ handlers changed with the result that a worker in musb_core.c got
> > scheduled far more frequently than needed.
> > 
> > I've included a patch below against 3.7, but i think it'll apply against
> > mainline.
> > [I apologize for any whitespace mangling.  I've also attached the patch.]
> > 
> > I'd like more eyeballs to tell me if this is right.  I'd also like to
> > know who I need to talk to to get this pushed into mainline.
> 
> Running the scripts/get_maintainer.pl script on your patch produces
> 
> Felipe Balbi <[email protected]> (maintainer:MUSB MULTIPOINT H...)
> Greg Kroah-Hartman <[email protected]> (supporter:USB SUBSYSTEM)
> [email protected] (open list:MUSB MULTIPOINT H...)
> [email protected] (open list)
> 
> Felipe Balbi (CC'ed) is the person who you should talk to.
> 
> While we're touching the subject of scripts, you should run the 
> scripts/checkpatch.pl script and fix errors and warnings before submitting 
> patches. Please see Documentation/SubmittingPatches.
> 
> Last (but not least) piece of advice, don't give up if you don't receive 
> replies to your patches. People are busy and mails fall to cracks from time to 
> time.
> 
> Felipe, apart from the coding style violation and the possibly missing 
> locking, what's your opinion on this ? Does the patch make sense ?

It's a duplication of the check which is already in musb_irq_work():

1742 static void musb_irq_work(struct work_struct *data)
1743 {
1744         struct musb *musb = container_of(data, struct musb, irq_work);
1745 
1746         if (musb->xceiv->state != musb->xceiv_old_state) {
1747                 musb->xceiv_old_state = musb->xceiv->state;
1748                 sysfs_notify(&musb->controller->kobj, NULL, "mode");
1749         }
1750 }

That does look better, but I'd need the check inside musb_irq_work() to
be removed and commit log would have to improve a bit.

ps: there's no missing locking, musb_stage0_irq() is called within
musb_interrupt() which is called within a locked IRQ handler.

-- 
balbi

_______________________________________________
linux-arm mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm
signature.asc (application/pgp-signature, 819 B)