Re: [PATCH 2/3] input: touchscreen: of: Register multitouch axes

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Thu, Nov 06, 2014 at 05:06:21PM +0100, Maxime Ripard wrote:
> Hi,
> 
> On Wed, Nov 05, 2014 at 09:56:35AM -0800, Dmitry Torokhov wrote:
> > On Wed, Nov 05, 2014 at 04:07:49PM +0100, Maxime Ripard wrote:
> > > So far, the DT parsing code was only setting up the regular input axes,
> > > completely ignoring their multitouch counter parts.
> > > 
> > > Fill them with the same parameters than the regular axes.
> > > 
> > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > ---
> > >  drivers/input/touchscreen/of_touchscreen.c | 12 +++++++++---
> > >  1 file changed, 9 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/input/touchscreen/of_touchscreen.c b/drivers/input/touchscreen/of_touchscreen.c
> > > index 74d6b0eb50ac..cf2a753edd96 100644
> > > --- a/drivers/input/touchscreen/of_touchscreen.c
> > > +++ b/drivers/input/touchscreen/of_touchscreen.c
> > > @@ -42,17 +42,23 @@ void touchscreen_parse_of_params(struct input_dev *dev)
> > >  
> > >  	maximum = of_get_optional_u32(np, "touchscreen-size-x");
> > >  	fuzz = of_get_optional_u32(np, "touchscreen-fuzz-x");
> > > -	if (maximum || fuzz)
> > > +	if (maximum || fuzz) {
> > >  		input_set_abs_params(dev, ABS_X, 0, maximum, fuzz, 0);
> > > +		input_set_abs_params(dev, ABS_MT_POSITION_X, 0, maximum, fuzz, 0);
> > 
> > Not all devices are multitouch so you shoudl not be setting multitouch
> > bits unconditionally.
> 
> Hmmm, right.
> 
> > In I think we should rely on driver to set capability bits properly and
> > then here test them and apply the readings. Probably also issue a
> > warning if we see max/fuzz setting but neither ABS_<N>/ABS_MT_<N>
> > capabilities.
> 
> What I was aiming at was to avoid DT parsing duplication for the
> !multitouch and multitouch axis. I don't think there's a way to copy
> the parameters.
> 
> The mt_init_slots might be a solution, but it does the copy the other
> way around: from the multitouch to the !multitouch axis, and without
> enabling it, which renders using both input_mt_init_slots and the
> of_touchscreen code together impossible.
> 
> Is there a way to just enable an axis without calling
> input_set_abs_params? Is __set_bit enough?

Hmm, not anymore I'm afraid as you still need absinfo being allocated,
but there is input_set_capability() that is basically a fancy
__set_bit(). If you set ABS_* with it that would mean that the device
has unspecified min/max/fuzz.

And then, in DT parsing code, I'd do:

	...read max and fuzz for an axis...
	if (max || fuzz) {
		if (!test_bit(ABS_N, dev->absbit) &&
		    !test_bit(ABS_MT_N, dev->absbit)) {
			dev_warn("... have parameters but axis not set up...");
		} else {
			if (test_bit(ABS_N...)
				input_set_abs_params(dev, ABS_N...);
			if (test_bit(ABS_MT_N...)
				input_set_abs_params(dev, ABS_MT_N...);
	}

Thanks.

-- 
Dmitry