Hi Jeff, On Tue, Dec 01, 2020 at 11:53:50PM -0600, Jeff LaBundy wrote: > 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.i I was thinking about allowing both "touch-" and "touchscreen-" for the allowed property names. > 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. That is fine, we can rename the file later if we decide to change the namespace. > 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. Yep, that sounds great, thank you. -- Dmitry
