Re: [PATCH v2 0/1] Input: xpad - Implement wireless controller LED setting and fix connect time LED setting

On Mon, 10 Dec 2012 02:58:25 -0800
Chris Moeller [off-list ref] wrote:

On Sun, 2 Dec 2012 23:43:18 -0800
Dmitry Torokhov [off-list ref] wrote:

quoted

On Fri, Nov 30, 2012 at 08:13:29PM -0800, Chris Moeller wrote:

quoted

On Fri, 30 Nov 2012 14:30:23 -0800
Dmitry Torokhov [off-list ref] wrote:

quoted

Hi Chris,

On Friday, November 30, 2012 01:54:06 PM Chris Moeller wrote:

quoted

I've submitted versions of this with prior patch sets, and
this part was never accepted, possibly because it depended on
other patches to work, or possibly because it wasn't so
cleanly organized. This time, I've split the LED setting
command off into its own static function, then call that on
controller connect and optionally on LED setting commands.
The static function does not include any locking, because
locking inside the function which receives the incoming
packets deadlocks the driver, and does not appear to be
necessary anyway.

It also removes all traces of the original bulk out function
which was designed for the purpose of setting the LED status
on connect, as I found that it actually does not work at all.
It appears to try to send the data, but nothing actually
happens to the controller LEDs.

Attached is a reply I sent to on 7/4/11 to which you
unfortunately never responded. The issue is that of force
feedback (rumble) and LED share the same URB then access to
that URB should be arbitrated. The attached message contains a
patch that attempts to implement that arbitration, could you
please try it out and see what changes are needed to make it
work?

Thanks.

So sorry I missed your reply. That's what I get for filtering the
mailing list messages past my inbox, then never following up on my
filter/folder set for replies to my own messages. I recall you
mentioned that potential race condition when I first submitted,
but I forgot to do anything about it. I'm glad at least one of us
has our stuff together. It seems to work just fine, but there may
be a force feedback issue with the following test program, where
it leaves the effect playing indefinitely after the program
terminates, and then the controller itself ceases to respond until
the module is removed and reloaded.

Just to confirm, you see this problem only with the patch being
discussed and do not see it without it, right?

Yeah, the problem only happens with this patch. Here's a silly mess
which fixes it. If you can think of a better way to handle pending
commands outside of the IRQ, be my guest. The problem seems to be that
it doesn't like sending further commands from inside the output irq
callback, so further commands are not sent, and the spinlock is left
locked or something. So it seems that it needs to be such that the
callback merely signals when the packet has completed sending, and the
actual queue must be handled outside of the irq, and somehow kindly
wait for the irq to complete for each command. Unless, you know,
there's some other way to queue up multiple commands without waiting
on each one to complete. I know bulk out doesn't work for the LED
setting command, at least. Anyway, here goes.

Disregard this patch, it locks up frequently. I'm working on something
else instead.