[linux-dvb] [PATCH] Allow budget-ci to only listen to certain IR devices

David Härdeman david at hardeman.nu
Thu Sep 28 12:07:20 CEST 2006


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




More information about the linux-dvb mailing list