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@xxxxxxxxxxx> 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 -- 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