Re: [PULL] http://linuxtv.org/hg/~awalls/v4l-dvb-misc
Andy Walls wrote:
The better is to rely on input_dev stuff, since they can easily be used by
ir-core
sysfs to provide device naming for loading keytables from userspace during
udev
handling.
OK, I don't see struct input_dev providing any storage for name and phys
strings. From vanilla-2.6.31-rc8/include/linux/input.h:
struct input_dev {
const char *name;
const char *phys;
[...]
and vanilla-2.6.31-rc8/drivers/input/input.c:input_register_device()
doesn't do any allocation. In fact it spits out useless default strings
like Unspecified device if input_dev-name is NULL.
Also vanilla-2.6.31-rc8/drivers/input/input.c:input_devices_seq_show(),
doesn't print any useful information if they are not set:
seq_printf(seq, N: Name=\%s\\n, dev-name ? dev-name : );
seq_printf(seq, P: Phys=%s\n, dev-phys ? dev-phys : );
So I don't see where the input layer is providing anything: storage
space nor automatic string generation.
That's true, but the drivers should be dynamically allocating the memory:
ir-phys = kasprintf(GFP_KERNEL, pci-%s/ir0, pci_name(pci));
or
ir-phys = kasprintf(GFP_KERNEL, usb-%s-%s, dev-bus-bus_name,
dev-devpath);
For the IR name, probably this would be enough:
ir-name = dev-name;
That's said, I agree that the better is to provide automatic string generation
at the ir-core, for both PCI and USB cases.
Of course, those names should be freed when unregistering the device.
Some of the V4L drivers use struct card_ir from
linux/include/media/ir-common.h:
struct card_ir {
struct input_dev*dev;
struct ir_input_state ir;
charname[32];
charphys[32];
[...]
Yes, i know. Legacy code. We should really remove name/phys from there
and use the pointers at input.h. As input layer changed, we should have
better patched the IR drivers to not duplicate data.
which has string storage that is too small for the card names in
cx23885-cards.c and also has a group of fields that don't make a lot of
sense for the RC-5 and NEC decoding state machines I've implemented.
In fact linux/drivers/media/video/cx88/cx88-input.c uses it's own struct
cx88_IR instead of card_IR:
struct cx88_IR {
struct cx88_core *core;
struct input_dev *input;
struct ir_input_state ir;
char name[32];
char phys[32];
[...]
So what is it you want me to do with this change? I don't know what
storage space for strings you want me to reuse.
Regards,
Andy
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://linuxtv.org/hg/~awalls/v4l-dvb-misc
On Sun, 2010-01-10 at 11:52 -0200, Mauro Carvalho Chehab wrote:
Andy Walls wrote:
On Sun, 2010-01-10 at 09:35 -0200, Mauro Carvalho Chehab wrote:
Andy Walls wrote:
Mauro,
If no one has any objections, please pull from
http://linuxtv.org/hg/~awalls/v4l-dvb-misc
for the following 12 changesets.
Of note:
02-04 are from Jean Delvare and fix up the cx23885 i2c routines
05-17 and 12 add and use a new v4l2_subdev core op for configuring I/O
pin muxes
08-10 are some minor cx23885 ir fixes noted when trying to get the TeVii
S470 working
10/12: cx23885: Convert from struct card_ir to struct cx23885_ir_input
for IR Rx
http://linuxtv.org/hg/~awalls/v4l-dvb-misc?cmd=changeset;node=aa62944baa92
Hmm... This doesn't sound right:
+struct cx23885_ir_input {
+ struct input_dev*dev;
+ struct ir_input_state ir;
+ charname[48];
+ charphys[48];
+
+ /* Cooked code processing */
+ int start; /* Allowed start symbols */
+ u32 addr;/* Expected remote address */
+ u32 last_code; /* last good cooked code seen
*/
+ int key_timeout; /* ms until we force a key up
*/
+ struct timer_list timer_keyup; /* timer for key release */
+
+ /* Raw code collection and construction */
+ int active; /* building code */
+ int last_bit; /* last bit seen */
+ u32 code; /* code under construction */
+};
Why are you creating a name[] and phys[] chars here? It should be using
the names already
defined at struct input_dev.
Well two reasons:
1. That's what the previous, common card ir struct did. (Not a good
reason of course.) When I needed to reimplement specific fields (in
anticipation of NEC decoding for the TeVii S470) I just carried them
over.
2. The strings in the old card ir struct were too short: the card names
in cx23885-cards.c are pretty long and would get truncated.
I'll reexamine if the strings in input_dev are long enough to do the
job, and get back to you.
The better is to rely on input_dev stuff, since they can easily be used by
ir-core
sysfs to provide device naming for loading keytables from userspace during
udev
handling.
OK, I don't see struct input_dev providing any storage for name and phys
strings. From vanilla-2.6.31-rc8/include/linux/input.h:
struct input_dev {
const char *name;
const char *phys;
[...]
and vanilla-2.6.31-rc8/drivers/input/input.c:input_register_device()
doesn't do any allocation. In fact it spits out useless default strings
like Unspecified device if input_dev-name is NULL.
Also vanilla-2.6.31-rc8/drivers/input/input.c:input_devices_seq_show(),
doesn't print any useful information if they are not set:
seq_printf(seq, N: Name=\%s\\n, dev-name ? dev-name : );
seq_printf(seq, P: Phys=%s\n, dev-phys ? dev-phys : );
So I don't see where the input layer is providing anything: storage
space nor automatic string generation.
Some of the V4L drivers use struct card_ir from
linux/include/media/ir-common.h:
struct card_ir {
struct input_dev*dev;
struct ir_input_state ir;
charname[32];
charphys[32];
[...]
which has string storage that is too small for the card names in
cx23885-cards.c and also has a group of fields that don't make a lot of
sense for the RC-5 and NEC decoding state machines I've implemented.
In fact linux/drivers/media/video/cx88/cx88-input.c uses it's own struct
cx88_IR instead of card_IR:
struct cx88_IR {
struct cx88_core *core;
struct input_dev *input;
struct ir_input_state ir;
char name[32];
char phys[32];
[...]
So what is it you want me to do with this change? I don't know what
storage space for strings you want me to reuse.
Regards,
Andy
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://linuxtv.org/hg/~awalls/v4l-dvb-misc
On Sun, 2010-01-10 at 11:52 -0200, Mauro Carvalho Chehab wrote:
Andy Walls wrote:
On Sun, 2010-01-10 at 09:35 -0200, Mauro Carvalho Chehab wrote:
Andy Walls wrote:
Mauro,
If no one has any objections, please pull from
http://linuxtv.org/hg/~awalls/v4l-dvb-misc
for the following 12 changesets.
Of note:
02-04 are from Jean Delvare and fix up the cx23885 i2c routines
05-17 and 12 add and use a new v4l2_subdev core op for configuring I/O
pin muxes
08-10 are some minor cx23885 ir fixes noted when trying to get the TeVii
S470 working
10/12: cx23885: Convert from struct card_ir to struct cx23885_ir_input
for IR Rx
http://linuxtv.org/hg/~awalls/v4l-dvb-misc?cmd=changeset;node=aa62944baa92
Hmm... This doesn't sound right:
+struct cx23885_ir_input {
+ struct input_dev*dev;
+ struct ir_input_state ir;
+ charname[48];
+ charphys[48];
+
+ /* Cooked code processing */
+ int start; /* Allowed start symbols */
+ u32 addr;/* Expected remote address */
+ u32 last_code; /* last good cooked code seen
*/
+ int key_timeout; /* ms until we force a key up
*/
+ struct timer_list timer_keyup; /* timer for key release */
+
+ /* Raw code collection and construction */
+ int active; /* building code */
+ int last_bit; /* last bit seen */
+ u32 code; /* code under construction */
+};
Why are you creating a name[] and phys[] chars here? It should be using
the names already
defined at struct input_dev.
Well two reasons:
1. That's what the previous, common card ir struct did. (Not a good
reason of course.) When I needed to reimplement specific fields (in
anticipation of NEC decoding for the TeVii S470) I just carried them
over.
2. The strings in the old card ir struct were too short: the card names
in cx23885-cards.c are pretty long and would get truncated.
I'll reexamine if the strings in input_dev are long enough to do the
job, and get back to you.
The better is to rely on input_dev stuff, since they can easily be used by
ir-core
sysfs to provide device naming for loading keytables from userspace during
udev
handling.
OK. Hold off on that whole pull request. That whole pull request was
the stable/ready part of what is in my cx23885-ir tree.
However, I just found what was wrong with my cx23885-ir tree code for IR
from an actual CX23885 chip. I'll just rework and update the whole
series, once I also get a change prepared and tested for the TeVii S470.
Regards,
Andy
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://linuxtv.org/hg/~awalls/v4l-dvb-misc
On Sun, 2010-01-10 at 09:35 -0200, Mauro Carvalho Chehab wrote:
Andy Walls wrote:
Mauro,
If no one has any objections, please pull from
http://linuxtv.org/hg/~awalls/v4l-dvb-misc
for the following 12 changesets.
Of note:
02-04 are from Jean Delvare and fix up the cx23885 i2c routines
05-17 and 12 add and use a new v4l2_subdev core op for configuring I/O pin
muxes
08-10 are some minor cx23885 ir fixes noted when trying to get the TeVii
S470 working
10/12: cx23885: Convert from struct card_ir to struct cx23885_ir_input for
IR Rx
http://linuxtv.org/hg/~awalls/v4l-dvb-misc?cmd=changeset;node=aa62944baa92
Hmm... This doesn't sound right:
+struct cx23885_ir_input {
+ struct input_dev*dev;
+ struct ir_input_state ir;
+ charname[48];
+ charphys[48];
+
+ /* Cooked code processing */
+ int start; /* Allowed start symbols */
+ u32 addr;/* Expected remote address */
+ u32 last_code; /* last good cooked code seen */
+ int key_timeout; /* ms until we force a key up */
+ struct timer_list timer_keyup; /* timer for key release */
+
+ /* Raw code collection and construction */
+ int active; /* building code */
+ int last_bit; /* last bit seen */
+ u32 code; /* code under construction */
+};
Why are you creating a name[] and phys[] chars here? It should be using the
names already
defined at struct input_dev.
Well two reasons:
1. That's what the previous, common card ir struct did. (Not a good
reason of course.) When I needed to reimplement specific fields (in
anticipation of NEC decoding for the TeVii S470) I just carried them
over.
2. The strings in the old card ir struct were too short: the card names
in cx23885-cards.c are pretty long and would get truncated.
I'll reexamine if the strings in input_dev are long enough to do the
job, and get back to you.
Regards,
Andy
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://linuxtv.org/hg/~awalls/v4l-dvb-misc
Andy Walls wrote:
On Sun, 2010-01-10 at 09:35 -0200, Mauro Carvalho Chehab wrote:
Andy Walls wrote:
Mauro,
If no one has any objections, please pull from
http://linuxtv.org/hg/~awalls/v4l-dvb-misc
for the following 12 changesets.
Of note:
02-04 are from Jean Delvare and fix up the cx23885 i2c routines
05-17 and 12 add and use a new v4l2_subdev core op for configuring I/O pin
muxes
08-10 are some minor cx23885 ir fixes noted when trying to get the TeVii
S470 working
10/12: cx23885: Convert from struct card_ir to struct cx23885_ir_input for
IR Rx
http://linuxtv.org/hg/~awalls/v4l-dvb-misc?cmd=changeset;node=aa62944baa92
Hmm... This doesn't sound right:
+struct cx23885_ir_input {
+ struct input_dev*dev;
+ struct ir_input_state ir;
+ charname[48];
+ charphys[48];
+
+ /* Cooked code processing */
+ int start; /* Allowed start symbols */
+ u32 addr;/* Expected remote address */
+ u32 last_code; /* last good cooked code seen */
+ int key_timeout; /* ms until we force a key up */
+ struct timer_list timer_keyup; /* timer for key release */
+
+ /* Raw code collection and construction */
+ int active; /* building code */
+ int last_bit; /* last bit seen */
+ u32 code; /* code under construction */
+};
Why are you creating a name[] and phys[] chars here? It should be using the
names already
defined at struct input_dev.
Well two reasons:
1. That's what the previous, common card ir struct did. (Not a good
reason of course.) When I needed to reimplement specific fields (in
anticipation of NEC decoding for the TeVii S470) I just carried them
over.
2. The strings in the old card ir struct were too short: the card names
in cx23885-cards.c are pretty long and would get truncated.
I'll reexamine if the strings in input_dev are long enough to do the
job, and get back to you.
The better is to rely on input_dev stuff, since they can easily be used by
ir-core
sysfs to provide device naming for loading keytables from userspace during udev
handling.
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
