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

Reply via email to