Re: [PATCH 9/9] musb_gadget_ep0: fix unhandled IRQ

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

David Brownell wrote:

David Brownell wrote:
The EP0 code routinely ignores interrupt at end of the data phase because of
musb_g_ep0_giveback() resetting the state machine to "idle, waiting for SETUP"
phase prematurely.

Signed-off-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
Does this depend on any preceding patches?
  Preceding patches don't touch this file, if you haven't noticed.

I don't mean just textually ... there are many kinds of dependency.
Like, maybe it's really only viable given other bugfixes.  And the
default assumption is that patches in a series depend on preceding
patches; but I suspected this one wouldn't.

  And you were right...

---
This fixes most of the unhandled gadget interrupts that generic_interrupt() and
davinci_interrupt() have been hiding from user by faking their result and only
emitting 5th level debug message (for what is clearly an error).
So, this should then remove the comment and DBG() from the
davinci_interrupt() and generic_interrupt() IRQ handlers,
and stop always returning IRQ_HANDLED.
   Well, that's another problem. But if you insist, I can do that.

It's all part of the same problem.  The reason that code exists
is because of the bug you say this fixes.  If it's still needed,
then this bug isn't fully fixed by $SUBJECT patch ...

I've already wrttien that it's not completely fixed (even in the original patch comment I said "majority of cases").

when such messages are cranked up
to a "serious impact on usability" level?  How did you
reproduce the problem?
By *not* ignoring the musb_interrupt() return value on OMAP-L137 -- and doing t for a good reason. :-(

OK.  I couldn't remember how common these were.

  They happended at end of *every* control transfer for me.

Common enough to notice, yes -- but I don't recall if they only
came out in stress tests, or in real-world loads.

  No stressing -- just the usual interchange during the device enumeration.

ISTR this seemed harmless, which is why its diagnostic was
low priority and the mess only got a "REVISIT" comment.
Which had never been revisited of course, until I had to look into it . :-)

I think there's a certain tendancy for folk to want to
take a break from this driver code after getting sufficiently
bloodied by the battle.  ;)

In my case it's usually forced break because they switch me to something other... like this damned MUSB thing (still better than KGDB though).

It's gotten much better over time, but the original codebase
was truly horrendous.

  I imagine...

So we're quite glad to see folk contributing fixes like yours!

  Well, someone has to do it... :-)
\
WBR, Sergei


--
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

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux