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;
> char name[32];
> char phys[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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
