Re: [PATCH 0/6] rc-core: ir-core to rc-core conversion

["David Härdeman" <david@hardeman.nu>]

On Thu, Nov 11, 2010 at 06:40:42PM -0500, Jarod Wilson wrote:
>On Thu, Nov 11, 2010 at 3:35 PM, David Härdeman <david@hardeman.nu> wrote:
>> On Thu, Nov 11, 2010 at 02:00:38PM -0200, Mauro Carvalho Chehab wrote:
>>>I like the idea of having an inlined function (like
>>>usb_fill_control_urb), to be sure that all mandatory fields are
>>>initialized by the drivers.
>>
>> I like the idea of having a function, let's call it
>> rc_register_device(), which makes sure that all mandatory fields are
>> initialized by the drivers :)
>
>rc_register_device(rc, name, phys, id); to further prevent duplicate
>struct members? :)

As I said before, that won't work with multiple input devices per rc
dev. And it's a poorly designed API (IMHO) which expects you to set a
few properties in a struct and then add a few more via a function call.

Notes wrt. a future multi-input support:

First, realize that the name/phys/id of an input device is primarily
used to distinguish them from each other in user-space. Which means
having a shared name/phys/id triplet for all input devices belonging to
one rc device only makes them pointless.

So, I think we'll want something similar to name/phys/id, but for the rc
device (can be exported via sysfs). Input name/phys/id can then be
derived from the rc device for each input subdev (or name could perhaps
be set to some user-friendly description of the actual remote control by
whichever user-space tool loads the corresponding keymap).

Example:
rc_dev->name = "FooBar IR-Masta 2000"
rc_dev->phys = "PNP0BAR/rc0"
(would be available from /sys/class/rc/rc0/{name|phys})

/* Not set by the driver, available with lsinput */
rc_dev->input_devs[0]->name = "FooBar IR-Masta 2000 Remote 1"
rc_dev->input_devs[0]->phys = "PNP0BAR/rc0/input0"
rc_dev->input_devs[0]->id.bustype = BUS_RC;
rc_dev->input_devs[1]->name = "FooBar IR-Masta 2000 Remote 2"
rc_dev->input_devs[1]->phys = "PNP0BAR/rc0/input1"
rc_dev->input_devs[1]->id.bustype = BUS_RC;

(Just an example, don't overanalyse the details)

As you see, the rc_dev->input_name/phys/id is just a stopgap measure for
now, and I certainly don't think future development will be helped by
moving any input related fields up to the rc_register_device() level
when they should instead go further "down".

>I still really like this interface change, even if its going to cause
>short-term issues for i2c devices. I think we just extend this as
>needed to handle the i2c bits.

Agreed.

-- 
David Härdeman