Re: HG pull http://jusst.de/hg/mantis-v4l-dvb

2010-03-09 Thread Mauro Carvalho Chehab 
Manu Abraham wrote:
 Mauro,
 
 Please pull from http://jusst.de/hg/mantis-v4l-dvb

This tree also needs rebase. Some of the patches were already applied:

# Mercurial patches imported from http://jusst.de/hg/mantis-v4l-dvb
hg_14410_stv090x_code_simplification.patch
hg_14411_stv090x_stv6110x_use_tuner_sleep_within_the_demodulator_control.patch
hg_14412_stv090x_use_gate_control_while_tuner_is_being_accessed.patch
hg_14413_budget_stv090x_stv6110x_initialize_the_demodulator_immediately_after.patch
hg_14414_stv090x_add_some_notes_about_the_internal_tuner_i_o_control.patch
hg_14415_mantis_remote_control_for_mantis.patch
hg_14416_mantis_hopper_use_module_device_table.patch
hg_14417_mantis_hopper_do_not_unregister_modules_twice.patch

Also, if just the last 3 patches are applied, lots of checkpacth errors are 
produced:

WARNING: line over 80 characters
#172: FILE: drivers/media/dvb/mantis/mantis_cards.c:223:
+   dprintk(MANTIS_ERROR, 1, ERROR: Mantis INPUT initialization 
failed %d, err);

WARNING: line over 80 characters
#355: FILE: drivers/media/dvb/mantis/mantis_dvb.c:292:
+   mantis-demux.dmx.remove_frontend(mantis-demux.dmx, 
mantis-fe_mem);

WARNING: line over 80 characters
#356: FILE: drivers/media/dvb/mantis/mantis_dvb.c:293:
+   mantis-demux.dmx.remove_frontend(mantis-demux.dmx, 
mantis-fe_hw);

ERROR: trailing whitespace
#489: FILE: drivers/media/dvb/mantis/mantis_input.c:57:
+^Isnprintf(mir-rc_name, sizeof(mir-rc_name), $

ERROR: code indent should use tabs where possible
#490: FILE: drivers/media/dvb/mantis/mantis_input.c:58:
+^I Mantis %s IR Receiver, mantis-hwconfig-model_name);$

ERROR: trailing whitespace
#491: FILE: drivers/media/dvb/mantis/mantis_input.c:59:
+^Isnprintf(mir-rc_phys, sizeof(mir-rc_phys), $

ERROR: code indent should use tabs where possible
#492: FILE: drivers/media/dvb/mantis/mantis_input.c:60:
+^I pci-%s/ir0, pci_name(mantis-pdev));$

WARNING: line over 80 characters
#499: FILE: drivers/media/dvb/mantis/mantis_input.c:64:
+   dprintk(MANTIS_ERROR, 1, Input device %s PCI: %s, mir-rc_name, 
mir-rc_phys);

WARNING: line over 80 characters
#551: FILE: drivers/media/dvb/mantis/mantis_input.c:105:
+   dprintk(MANTIS_ERROR, 0, RC Input Sendkey:%d %02x\n, i, 
buf[i]);

ERROR: space required before the open parenthesis '('
#588: FILE: drivers/media/dvb/mantis/mantis_uart.c:106:
+   if(config-uart_work) {

WARNING: braces {} are not necessary for single statement blocks
#588: FILE: drivers/media/dvb/mantis/mantis_uart.c:106:
+   if(config-uart_work) {
+   config-uart_work(mantis, buf);
+   }

ERROR: code indent should use tabs where possible
#592: FILE: drivers/media/dvb/mantis/mantis_uart.c:110:
+/* reenable interrupt */$

ERROR: code indent should use tabs where possible
#593: FILE: drivers/media/dvb/mantis/mantis_uart.c:111:
+mmwrite(mmread(MANTIS_INT_MASK) | 0x800, MANTIS_INT_MASK);$

ERROR: code indent should use tabs where possible
#594: FILE: drivers/media/dvb/mantis/mantis_uart.c:112:
+mmwrite(mmread(MANTIS_UART_CTL) | MANTIS_UART_RXINT, MANTIS_UART_CTL);$

WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
#670: FILE: drivers/media/dvb/mantis/mantis_vp1041.c:102:
+EXPORT_SYMBOL_GPL(ir_codes_mantis_vp1041_table);

WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
#767: FILE: drivers/media/dvb/mantis/mantis_vp2033.c:107:
+EXPORT_SYMBOL_GPL(ir_codes_mantis_vp2033_table);

WARNING: EXPORT_SYMBOL(foo); should immediately follow its function/variable
#850: FILE: drivers/media/dvb/mantis/mantis_vp2040.c:93:
+EXPORT_SYMBOL_GPL(ir_codes_mantis_vp2040_table);

total: 8 errors, 9 warnings, 798 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

HG pull http://jusst.de/hg/mantis-v4l-dvb

2010-03-07 Thread Manu Abraham 
Mauro,

Please pull from http://jusst.de/hg/mantis-v4l-dvb

for the following changes

changeset 14416
mantis, hopper: do not unregister modules twice
http://jusst.de/hg/mantis-v4l-dvb/rev/0e6ee2a233f0

changeset 14415
Mantis, hopper: use MODULE_DEVICE_TABLE
http://jusst.de/hg/mantis-v4l-dvb/rev/3731f71ed6bf

changeset 14414
Mantis: Remote Control for Mantis
http://jusst.de/hg/mantis-v4l-dvb/rev/ad8b00c9edc2
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Pull http://jusst.de/hg/mantis-v4l-dvb

2010-02-12 Thread Mauro Carvalho Chehab 
Manu Abraham wrote:
 On Thu, Feb 11, 2010 at 11:44 PM, Mauro Carvalho Chehab
 mche...@infradead.org wrote:
 Manu Abraham wrote:

 changeset 14166
 http://jusst.de/hg/mantis-v4l-dvb/rev/c2391fa88112
 Mantis: Remote Control for Mantis
 +   ir_input_init(rc_dev, rc_state, IR_TYPE_OTHER);

 ...

 +struct ir_scancode_table ir_codes_mantis_vp2040_table = {
 +   .scan = ir_codes_mantis_vp2040,
 +   .size = ARRAY_SIZE(ir_codes_mantis_vp2040),
 +};
 +EXPORT_SYMBOL_GPL(ir_codes_mantis_vp2040_table);

 The non-declaration of the IR protocol and the definition of
 incomplete IR codes is deprecated. Please, create a table with
 the IR protocol type and add the IR address to each IR scancode
 at the ir codes tables.

 Please look at em28xx-input to see how to implement it right.
 The change is minimal:
 - at the driver, you'll need to make sure that you're
  using the full IR scan code, instead of just the 8 bits for IR cmd;

 - at the table: you need to use IR address+command scan code,
  and declare the protocol associated with the table;
 
 
 Huh ? em28xx uses a I2C based implementation for the IR.
 There's no address associated with the IR stuff on the mantis, which
 is on something say like COM 0/1, which is more similar, rather than
 an I2C device or a polling based device.

This is true only for the old devices. The em2860/em2880 devices support
NEC and RC5. Newer chips (like em2884) also support RC6.

Anyway, the changes are not in the way the device communicates with the chip, 
but
the way it is presented to IR core, and how the keycodes are decoded.

If you take a look on em28xx-input, you'll see, for example:

static int default_polling_getkey(struct em28xx_IR *ir,
  struct em28xx_ir_poll_result *poll_result);

This routine gets 4 bytes from the device and stores both the RC address and
RC command (at rc_address and rc_data[0] fields).

The routine that generates the keypress events is:

static void em28xx_ir_handle_key(struct em28xx_IR *ir);

As the driver still work with the legacy keytables, it is easy to check where's
the difference between handling an incomplete or a complete keycode:

if (ir-full_code)
ir_input_keydown(ir-input, ir-ir,
 poll_result.rc_address  8 |
 poll_result.rc_data[0]);
else
ir_input_keydown(ir-input, ir-ir,
 poll_result.rc_data[0]);


So, basically, new drivers and new keycodes should use the entire RC code
(address + data) to the input subsystem.
 
  Also the protocol is not something that can be seen externally,  It
 is all internal to the firmware on the micro. The micro auto detect's
 the relevant protocol. ie, what it outputs is all data, no messages.

That's even better. Yet, if you're using just the IR cmd, people won't
be able to use another IR with the device.
 
 I don't follow what you tend to imply ..
 
 
 - at the driver: instead of using IR_TYPE_OTHER, just pass the
 value that comes from the IR table.

 A correct implementation allows the replacement of the IR by
 universal ones, at userspace, the key re-definition and, if
 the driver supports more protocol, the protocol changes,
 all provided by the new IR input class.
 
 
 
 Ah, i now understand what you are trying to say.. Unfortunately you
 can't do a protocol conversion or anything likewise on the Mantis IR
 device. It is all in hardware and the protocol type is all auto
 detected. All what you can change is the UART settings such as BPS,
 Parity and STOP bit. There's nothing the user can do to change
 anything in there.
 
 To put it short: whatever remote you use, if it is a supported remote
 by the firmware, it will output the relevant keycode directly, not
 messages such that the message needs to be decoded further.

Yes, but the IR core allows replacing the keycode - scancode from
the driver. This allows some usecases, like:
- using a different IR;
- using the same IR, but customizing the keys;
- using a different IR to control several different devices,
  where only some keys will be handled by the device.

 I think I need to add add a macro to export the IR table, replacing
 all *_table definitions, in order to avoid patches submitted with
 the legacy way (and to remove a false-positive on checkpatch.pl).

 Also there are some coding style troubles here, including bad
 whitespaces. By running make whitespace, you'll be clearing most
 of the warnings.
 
 
 Ok, I will take a look, tomorrow evening.
 
 Regards,
 Manu


-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Pull http://jusst.de/hg/mantis-v4l-dvb

2010-02-11 Thread Manu Abraham 
Mauro,

Please pull the following changes:

changeset 14168
http://jusst.de/hg/mantis-v4l-dvb/rev/9804df36159c
mantis, hopper: do not unregister modules twice

changeset 14167
http://jusst.de/hg/mantis-v4l-dvb/rev/6f3e1db2432a
Mantis, hopper: use MODULE_DEVICE_TABLE

changeset 14166
http://jusst.de/hg/mantis-v4l-dvb/rev/c2391fa88112
Mantis: Remote Control for Mantis

changeset 13880
http://jusst.de/hg/mantis-v4l-dvb/rev/864d3c4d8312
[Mantis] Add support for another revision of the Terratec Cinergy C PCI HD

Regards,
Manu
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Pull http://jusst.de/hg/mantis-v4l-dvb

2010-02-11 Thread Mauro Carvalho Chehab 
Manu Abraham wrote:

 changeset 14166
 http://jusst.de/hg/mantis-v4l-dvb/rev/c2391fa88112
 Mantis: Remote Control for Mantis

+   ir_input_init(rc_dev, rc_state, IR_TYPE_OTHER);

...

+struct ir_scancode_table ir_codes_mantis_vp2040_table = {
+   .scan = ir_codes_mantis_vp2040,
+   .size = ARRAY_SIZE(ir_codes_mantis_vp2040),
+};
+EXPORT_SYMBOL_GPL(ir_codes_mantis_vp2040_table);

The non-declaration of the IR protocol and the definition of
incomplete IR codes is deprecated. Please, create a table with
the IR protocol type and add the IR address to each IR scancode
at the ir codes tables.

Please look at em28xx-input to see how to implement it right.
The change is minimal: 
- at the driver, you'll need to make sure that you're 
  using the full IR scan code, instead of just the 8 bits for IR cmd;

- at the table: you need to use IR address+command scan code,
  and declare the protocol associated with the table;

- at the driver: instead of using IR_TYPE_OTHER, just pass the
value that comes from the IR table.

A correct implementation allows the replacement of the IR by
universal ones, at userspace, the key re-definition and, if
the driver supports more protocol, the protocol changes,
all provided by the new IR input class.

I think I need to add add a macro to export the IR table, replacing
all *_table definitions, in order to avoid patches submitted with
the legacy way (and to remove a false-positive on checkpatch.pl).

Also there are some coding style troubles here, including bad
whitespaces. By running make whitespace, you'll be clearing most
of the warnings. 



-- 

Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Pull http://jusst.de/hg/mantis-v4l-dvb

2010-02-11 Thread Manu Abraham 
On Thu, Feb 11, 2010 at 11:44 PM, Mauro Carvalho Chehab
mche...@infradead.org wrote:
 Manu Abraham wrote:

 changeset 14166
 http://jusst.de/hg/mantis-v4l-dvb/rev/c2391fa88112
 Mantis: Remote Control for Mantis

 +       ir_input_init(rc_dev, rc_state, IR_TYPE_OTHER);

 ...

 +struct ir_scancode_table ir_codes_mantis_vp2040_table = {
 +       .scan = ir_codes_mantis_vp2040,
 +       .size = ARRAY_SIZE(ir_codes_mantis_vp2040),
 +};
 +EXPORT_SYMBOL_GPL(ir_codes_mantis_vp2040_table);

 The non-declaration of the IR protocol and the definition of
 incomplete IR codes is deprecated. Please, create a table with
 the IR protocol type and add the IR address to each IR scancode
 at the ir codes tables.

 Please look at em28xx-input to see how to implement it right.
 The change is minimal:
 - at the driver, you'll need to make sure that you're
  using the full IR scan code, instead of just the 8 bits for IR cmd;

 - at the table: you need to use IR address+command scan code,
  and declare the protocol associated with the table;


Huh ? em28xx uses a I2C based implementation for the IR.
There's no address associated with the IR stuff on the mantis, which
is on something say like COM 0/1, which is more similar, rather than
an I2C device or a polling based device.

 Also the protocol is not something that can be seen externally,  It
is all internal to the firmware on the micro. The micro auto detect's
the relevant protocol. ie, what it outputs is all data, no messages.


I don't follow what you tend to imply ..


 - at the driver: instead of using IR_TYPE_OTHER, just pass the
 value that comes from the IR table.

 A correct implementation allows the replacement of the IR by
 universal ones, at userspace, the key re-definition and, if
 the driver supports more protocol, the protocol changes,
 all provided by the new IR input class.



Ah, i now understand what you are trying to say.. Unfortunately you
can't do a protocol conversion or anything likewise on the Mantis IR
device. It is all in hardware and the protocol type is all auto
detected. All what you can change is the UART settings such as BPS,
Parity and STOP bit. There's nothing the user can do to change
anything in there.

To put it short: whatever remote you use, if it is a supported remote
by the firmware, it will output the relevant keycode directly, not
messages such that the message needs to be decoded further.


 I think I need to add add a macro to export the IR table, replacing
 all *_table definitions, in order to avoid patches submitted with
 the legacy way (and to remove a false-positive on checkpatch.pl).

 Also there are some coding style troubles here, including bad
 whitespaces. By running make whitespace, you'll be clearing most
 of the warnings.


Ok, I will take a look, tomorrow evening.

Regards,
Manu
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html