On Wednesday 11 February 2009, Sergei Shtylyov wrote:
> Hello.
> 
> 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 <[email protected]>
> >>     
> >
> > 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.


> >> ---
> >> 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'm not sure I'd agree this is a "must fix for 2.6.29" bug.
> > What problems does it cause ... other than a low priority
> > debug message appearing,
> 
>    Low priority debug message where the error message was more 
> appropriate, you mean? :-)

If it caused no problems, it was no error.  That's
why it's a low priority debug message.  :)


> > 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.  Common
enough to notice, yes -- but I don't recall if they only
came out in stress tests, or in real-world loads.


> > 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.  ;)

It's gotten much better over time, but the original codebase
was truly horrendous.  So we're quite glad to see folk
contributing fixes like yours!

- Dave


 
> > - Dave
> >   
> 
> WBR, Sergei
> 
> 
> 
> 



_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to