On Thu, Dec 10, 2009 at 11:19:18AM -0200, Henrique de Moraes Holschuh wrote:
> On Wed, 09 Dec 2009, Dmitry Torokhov wrote:
> > > Still, please look at the patch below... Would something like this be a
> > > cleaner API? It is certainly more obvious, and it is cleaner on the driver
> > > side (one function call does everything, instead of a call to
> > > input_set_capability plus a call to whatever the driver needs to issue the
> > > initial EV_SW event)...
> > >
> >
> > Yes, I think it is a good idea. However why don't we change it to:
> >
> > input_setup_event(dev, type, code, value)
> > {
> > input_set_capability(...);
> > input_event(...);
> > }
> >
> > So it would work for everything (who knows, maybe down the road some
> > driver wants to init its ABS axes properly and so on)?
>
> Well, we already have input_set_abs_params, I'd say that's the correct place
> to init the axes... and you'd compile-break any drivers that need love to
> properly init the axes, so it is a win-win situation as it would make sure
> the patch is feature-complete (i.e. converts all drivers to the new call and
> makes sure they all init their abs params properly).
Right, input_set_abs_params()... No, I don't really want to go through
all touchpad, touchscreen and joystick drivers at once, thank you vey
much ;) Besides, for the vast majority of uses 0 is the proper initial
value for absolute axis.
> The only other event that needs initialization are the switches, so I'd
> argue that we might as well use my proposed patch, which is specific and
> more lightweight, and convert the in-tree drivers to it. A few might be
> missing the before-registering input_event...
>
As I think about it even more I think we should leave it asd is. You
should just split setting up the device's capabilities and seeding of
the initial values. Given that you need to re-seed the values upon
resume - and there you do need to use input_event() - just create a
function that queries the hardwware and sends the events and invoke it
once upon registration and also resume hardler.
> BTW, looking at input.h, wouldn't it be better to force the init functions
> to be run before registering the input device, doing a BUG_ON() if they're
> misused?
What function are you referring to? Input devices are in useable state
as returned by input_allocate_device().
> I'd also suggest a BUG_ON(!dev), or at least an if (!dev) return
> -EINVAL; to the top of input_register_device(...)...
>
Nah, just let it crash (the same effect as BUG_ON really).
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
