On Thu, September 28, 2006 1:31, Darren Salt said: > <URL:http://www.youmustbejoking.demon.co.uk/progs.linux.html#dvb> > > You might like to look at them since you evidently have functioning > hardware with which you can test & debug them. Ok, I've read through the patches...and the overall direction seems to be good. I've added per-patch comments below: ... ir_01_ir-common_handle_repeat.patch Seems like a good idea. Perhaps the debounce-logic should be moved to ir-common as well since it is a problem common to all IR input drivers? ir_02_budget-ci_groundwork_and_avoid_name_truncation.patch ir_03_budget-ci_EVIOCGPHYS.patch Looks ok ir_04_budget-ci_error_and_shutdown_fixups_and_verbosity.patch No objections to the patch, two comments though: The patch reverses the order of tasklet_kill() and msp430_ir_deinit(). Are you sure that it is correct to deinit the msp430 before killing the tasklet? Nitpick: input_event(...EV_KEY...) could be changed to input_report_key() ir_05_budget-ci_kzalloc_and_more_verbosity.patch Looks ok ir_06_budget-ci_use_ir-common_and_RC5_addresses.patch Not ok, the "address" (rc5 device) decoding logic will trigger on invalid data as well (0x80 <= data <= 0xc0), which the MSP430 will produce from time to time. In addition there is no logic to read all three bytes and to compare command1 and command2, meaning that the correlation between rc5 device and rc5 command may be wrong (the bytes can arrive in any order). For example, if you receive three bytes from the ir chip: data(device, rc5dev=30) data(command1, rc5cmd=18) data(device, rc5dev=22) Did you just receive command 18 to device 22 or command 18 to device 30? I like that it uses the RC5 toggle bit though, and that it only listens for device 0x1f per default (I had no idea what device code the remotes usually used so I had the reverse logic in my patch). ir_07_cx88xx_name_truncation_and_use_RC5_addresses_for_Hauppauge.patch ir_08_cx88-input_fix_repeat.patch ir_09_cx88-input_prevent_input_layer_repeat_handler.patch No comments, unfamiliar hardware... ir_10_ir-common_compressed_keymaps.patch ir_11_ir-common_optimise_keymap_sizes.patch Honestly, this smells of over-engineering. Compressed, compile-time generated keymaps using a perl-to-c script? I'd rather expect that all keymaps would be removed from the kernel (long term of course) and moved to an external program which loads the proper keymap. This would mean that only the used keymap would take up space and it would be trivial to add new keymaps without hacking kernel code. ... So how would you like to proceed with this? It would be nice to cooperate in order to have the added functionality commited to the dvb repo. Perhaps it would be a good idea to try to get patches 1 - 5 merged first and I can try to update the RC5 command decoding of your sixth patch? That way the first set of changes will also have some time to get some testing before moving along with more changes... -- David Härdeman _______________________________________________ linux-dvb mailing list linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
