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;
> >> + char name[48];
> >> + char phys[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;
char name[32];
char phys[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
