Re: [ibm-acpi-devel] [PATCH 10/10] thinkpad-acpi: sync input device EV_SW state directly
On Thu, Dec 10, 2009 at 10:46:02PM -0200, Henrique de Moraes Holschuh wrote:
> On Thu, 10 Dec 2009, Dmitry Torokhov wrote:
> > 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.
>
> Heh.
>
> > > 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
>
> It needs to be documented somewhere, at least.
>
Something like the patch below?
> > > 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
>
> The ones proposed in this subthread, which shouldn't be used after
> registering.
>
> > as returned by input_allocate_device().
>
> Unless, of course, someone screws up verifying if input_alocate_device
> returned NULL...
>
Then it will crash way before registering, as soon as they try to set
name, phys and so on.. Just not worh it.
--
Dmitry
Input: document use of input_event() function
From: Dmitry Torokhov
Signed-off-by: Dmitry Torokhov
---
drivers/input/input.c | 10 --
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index e097176..6240c7a 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -302,9 +302,15 @@ static void input_handle_event(struct input_dev *dev,
* @value: value of the event
*
* This function should be used by drivers implementing various input
- * devices. See also input_inject_event().
+ * devices to report input events. See also input_inject_event().
+ *
+ * NOTE: input_event() may be safely used right after input device was
+ * allocated with input_allocate_device(), even before it is registered
+ * with input_register_device(), but the event will not reach any of the
+ * input handlers. Such early invocation of input_event() may be used
+ * to 'seed' initial state of a switch or initial position of absolute
+ * axis, etc.
*/
-
void input_event(struct input_dev *dev,
unsigned int type, unsigned int code, int value)
{
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH 10/10] thinkpad-acpi: sync input device EV_SW state directly
On Thu, 10 Dec 2009, Dmitry Torokhov wrote: > 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. Heh. > > 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 It needs to be documented somewhere, at least. > > 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 The ones proposed in this subthread, which shouldn't be used after registering. > as returned by input_allocate_device(). Unless, of course, someone screws up verifying if input_alocate_device returned NULL... > > 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). There's a difference, and it is not small if it happens outside of a driver developer's test kernel. Not that I'd expect anyone to ship code so broken, that would manage to input_register_device(NULL), so I don't have any strong feelings on this. -- "One disk to rule them all, One disk to find them. One disk to bring them all and in the darkness grind them. In the Land of Redmond where the shadows lie." -- The Silicon Valley Tarot Henrique Holschuh -- Return on Information: Google Enterprise Search pays you back Get the facts. http://p.sf.net/sfu/google-dev2dev ___ ibm-acpi-devel mailing list ibm-acpi-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH 10/10] thinkpad-acpi: sync input device EV_SW state directly
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
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
Re: [ibm-acpi-devel] [PATCH 10/10] thinkpad-acpi: sync input device EV_SW state directly
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).
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...
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? I'd also suggest a BUG_ON(!dev), or at least an if (!dev) return
-EINVAL; to the top of input_register_device(...)...
--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
--
Return on Information:
Google Enterprise Search pays you back
Get the facts.
http://p.sf.net/sfu/google-dev2dev
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel
