On Mon, May 16, 2011 at 12:37 PM, Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx> wrote: > It is OK if patches are dependent upon each other. In this case I'd > recommend adding XBox360 Wireless rumble first, keeping the rest of the > driver structure, and then reworking LED support, which requires more > changes. Implementing rumble alone for the 360 Wireless pad will probably be a lot easier, and may even make it in. I'll step up to do that, assuming you don't plan to just take the command structure I posted and post a patch yourself. Maybe a lack of hardware prevents this, or maybe this should be a collaborative effort and I can do it? Hmm... > As long as the code was not used I think this should be OK. Yeah, I only used the command structure, which I presume belongs to Microsoft anyway, since it's their hardware. > I'd prefer the "command" calculation be hidden in > xpad_send_led_command(), i.e. it should accept led number and figure out > the proper command byte based on device type. Yeah, I can do that too. > This should probably be: > > if (xpad->xtype == XTYPE_UNKNOWN) Makes sense. I was only accounting for the possibility of other device types making it into the known devices set and making it through that condition check. Of course, that's not a problem at this point. > No, this conditional locking will not do. What happens if > xpad_send_led_command() gets called from both xpad360w_process_packet() > and xpad_led_set()? There is also bigger problem - we are using the same > URB/buffers as FF playback so we need to make sure we do not stomp on > each other toes. Yeah, maybe I should have made another IRQ output buffer for the LED commands. I just could not make the bulk output work at all. Although I'm not sure if that matters, if both the external LED interface and the FF playback lock the output buffer before using it. Also, the reason for the conditional locking is because the LED setting function needs to be called from within the driver itself for the player assignment that occurs when a wireless pad is added. Locking in that case causes the driver to hang when a wireless pad is connected. If you'll check, the condition to lock is cleared when called from within the driver lock in that case, but set so it always attempts to lock when it's called from the external interface functions. There's probably a better way of doing that, though. -Chris -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html