On Mon, 12 Mar 2012, Michal Malý wrote: > this is sort of a response to Simon Wood's recent G27 LED's patch but > since the patch touches a completely different part of the Logitech > driver I submitted it separately. Michal, thanks for the patch. > The main goal of this patch is to allow for some custom device specific > properties like the rotation range to be stored as private driver data. > Up until now the hid-lg treated the pointer to driver_data as unsigned > long as stored some device quirks there. Because of the scope of the > changes, Simon's implementation of the G27 LEDs code is unfortunately > incompatible with this patch because it uses the linked list I'm trying > to get rid of. I hope Simon will be refreshing his patch to use LEDS_CLASS anyway. > The patch also introduces some spinlocking to hid-lg and hid-lg4ff to > (hopefully) prevent any nasty race conditions you were concerned about. >From a cursory look the locking looks good (why do you need to disable IRQs while taking the lock? It doesn't seem to be taken in IRQ context anywhere, right?). But it's somewhat difficult to review, as it's inter-mixed with other changes. Could you please split your patch in two independent patches, one adding proper locking, and second one implementing the drv_data change? Those changes don't have anything in common in principle, so it makes sense to have them separated. Thanks. -- Jiri Kosina SUSE Labs -- 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
