Hi Dmitry,
On Mon, Nov 30, 2020 at 11:01:06PM -0800, Dmitry Torokhov wrote:
> 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_()?
Sure thing, I can move it. I guess we want to do the same for the binding
too? There are only a handful of other bindings that will need references
to touchscreen.yaml updated with a new relative path.
I'm hesitant to rename the API because we still need to support bindings
that start with touchscreen-* and having an API with different namespace
seems inconsistent. How about I volunteer the following for this series:
1. Move of_touchscreen.c to drivers/input, and rename it to touchscreen.c
since it is not actually related to OF at this point. This would match
the header file too.
2. Update its introductory comments from:
"Generic DT helper functions for touchscreen devices"
to:
"Generic helper functions for touchscreens and other two-dimensional
pointing devices"
3. Move touchscreen.* from bindings/input/touchscreen to bindings/input,
and update the handful of touchscreen bindings that assume a relative
path to touchscreen.yaml.
Let me know if this seems like a reasonable compromise.
>
> Thanks.
>
> --
> Dmitry
Kind regards,
Jeff LaBundy
