I demand that David Härdeman may or may not have written... > 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? Probably, yes. I'd prefer to have it enabled at the request of the dependent driver, though, until the migration is completed. [snip] > 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? Uncertain. If you think that that shouldn't be done, that's fine... > Nitpick: input_event(...EV_KEY...) could be changed to > input_report_key() So long as repeat events can be properly handled, I don't see any problem. [snip] > 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. Fair enough... > 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). There are *three* bytes? > 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? My use of two bytes is based on details worked out from the output of an Nova-T (no longer in use; suspected broken) - there seemed to be missed interrupts, which is probably why I've not noticed the extra data (that motherboard has failed since). > 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). I don't know what other remote controls (from other vendors) use. This kind of thing should be added, where known, per PCI ID. > 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... No problem; somebody else can have a look :-) > 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? Text-to-C perl script, surely. ;-) The primary goal of that pair of patches is to provide a better run-time keymap size for any IR input device (whose driver uses ir-common). Memory says that this will mostly result in 64-entry keymaps, but there may be the occasional 32-entry keymap; and I recall there being a 256-entry keymap for one device whose driver hadn't been converted to ir-common (no idea if it has since been converted) - better that keymaps can have sizes suitable for the device, rather than allocate 2 or possibly 4 times what's actually needed. The compression is a nice extra which it makes sense to do at the same time. Even without this, we'd still need info on the size of the keymap... > 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. Agreed. We'd need a way to tell ir-common the proper size of the keymap (for those non-RC5 devices) and we'd need the external program long before the in-kernel keymaps are removed. Something like one Debian release cycle, perhaps :-) > ... > 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? Probably, yes. It's probably better that you post them, given that you have relevant working hardware, with any necessary changes and the extra signed-off-by lines. Anybody who wants to compare - well, I don't plan on removing my patches from my website for some time yet... > That way the first set of changes will also have some time to get some > testing before moving along with more changes... No problem with that here :-) -- | Darren Salt | linux or ds at | nr. Ashington, | Toon | RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army | + Output *more* particulate pollutants. BUFFER AGAINST GLOBAL WARMING. Keep emotionally active. Cater for your favourite neurosis. _______________________________________________ linux-dvb mailing list linux-dvb@xxxxxxxxxxx http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
