Re: [PATCH 2/2] input: Add support for Azoteq IQS626A

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Jeff LaBundy <jeff@labundy.com>
Cc: robh+dt@kernel.org, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org
Subject: Re: [PATCH 2/2] input: Add support for Azoteq IQS626A
Date: Mon, 30 Nov 2020 23:01:06 -0800	[thread overview]
Message-ID: <20201201070106.GS2034289@dtor-ws> (raw)
In-Reply-To: <20201124001516.GA6249@labundy.com>

On Mon, Nov 23, 2020 at 06:15:16PM -0600, Jeff LaBundy wrote:
> Hi Dmitry,
> 
> Thank you for taking a look.
> 
> On Sun, Nov 22, 2020 at 11:03:07PM -0800, Dmitry Torokhov wrote:
> > Hi Jeff,
> > 
> > On Sun, Nov 22, 2020 at 04:39:08PM -0600, Jeff LaBundy wrote:
> > > +
> > > +		if ((sys_reg->active & tp_mask) == tp_mask)
> > > +			input_set_abs_params(iqs626->trackpad,
> > > +					     ABS_X, 0, 255, 0, 0);
> > > +		else
> > > +			input_set_abs_params(iqs626->trackpad,
> > > +					     ABS_X, 0, 128, 0, 0);
> > > +#ifdef CONFIG_TOUCHSCREEN_PROPERTIES
> > > +		touchscreen_parse_properties(iqs626->trackpad, false,
> > > +					     &iqs626->prop);
> > > +#endif /* CONFIG_TOUCHSCREEN_PROPERTIES */
> > 
> > This should not be separately selectable from CONFIG_INPUT, so there is
> > not need to have this guard.
> > 
> > The reason it is a separate symbol is historical - it used to depend on
> > OF in addition to INPUT. I suppose I can drop it now.
> 
> Without these guards, the build fails if CONFIG_INPUT_TOUCHSCREEN=n and
> I felt it too heavy-handed to add a 'depends on' for what is ultimately
> a corner-case of sorts for this device.

Ah, I missed the fat that we got outside of the
drivers/input/toucscreen.

> 
> The touchscreen helpers are useful for more than just touchscreens, and
> we can extend them to all of input with something like the patch below.
> If it looks OK to you, I can insert it into v2 after I collect feedback
> from Rob for the binding.

Yes, I guess we should move into core. Can you move the file into
drivers/input and maybe we should rename it into touch-properties.c? And
start renaming the API form touchscreen_*() to touch_()?

Thanks.

-- 
Dmitry