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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
