On Mon, Aug 23, 2010 at 08:19:44PM -0700, James wrote: > On 08/19/2010 05:10 PM, Greg KH wrote: >> On Thu, Aug 19, 2010 at 04:28:39PM -0700, James wrote: >> > ... >>> The above is enabled via CONFIG_TOUCHSCREEN_CY8CTMG110_MULTIPLE_INPUT >>> with the default set to 'Y' (to enable current Qt 4.7 based applications >>> to leverage this capability) >>> > ... >>> +config TOUCHSCREEN_CY8CTMG110_MULTIPLE_INPUT >>> + bool "cy8ctmg110 multiple interface support" >>> + default y >>> >> If it's default y, why even have a config option? >> > So that it can be turned off if someone doesn't want that behavior.
Why would they not want this? Why make it a config option at all? >> And note, we don't do default y for new config options, unless you can't >> boot your machine without it. >> > But it is 'Y' as a sub-option of the TOUCHSCREEN_CY8CTMG110: Doesn't matter. > + depends on TOUCHSCREEN_CY8CTMG110 > > so unless the user says Y/m to TOUCHSCREEN_CY8CTMG110, the MULTIPLE_INPUT > won't be enabled, correct? True, but again, it doesn't matter. Either default to 'N' or just don't have a config option. >>> --- a/drivers/input/touchscreen/cy8ctmg110_ts.c >>> +++ b/drivers/input/touchscreen/cy8ctmg110_ts.c >>> @@ -81,8 +81,13 @@ struct ts_event { >>> * The touch driver structure. >>> */ >>> struct cy8ctmg110 { >>> +#ifdef CONFIG_TOUCHSCREEN_CY8CTMG110_MULTIPLE_INPUT >>> + struct input_dev *input[MAX_FINGERS]; >>> + char phys[MAX_FINGERS][32]; >>> +#else >>> struct input_dev *input; >>> char phys[32]; >>> +#endif >>> >> No #ifdefs in .c files please. >> > I'll work on an improvement. Long term, I suspect this patch will > disappear in favor of a full multi-touch-protocol implementation. However, > if you want to deploy an entire multi-touch stack *now*, user space doesn't > use the multi-touch protocol, so we're stuck with having to write the > driver to the legacy mechanism. Doesn't that sound like a userspace bug? Why are you trying to fix it in the kernel? > A positive attribute of placing that logic in #ifdef's is that it clearly > isolates that code path within the .c file such that a maintainers can more > readily remove that code without having oddly obfuscated static inline > functions floating around (I'm not arguing for #ifdef approaches in > general, just noting a benefit) One might just redefine MAX_FINGERS to 1 > and remove all of the #ifdef logic... but in the real multi-touch-protocol > solution, we won't need to have code capable of creating one input device > per contact point. Sorry, but no, no #ifdefs please. That way lies madness in the long run. And if someone really wanted to revert these changes, a simple 'git revert' would be all that is needed sometime in the future. Don't think that adding #ifdef would help that effort. thanks, greg k-h _______________________________________________ MeeGo-dev mailing list MeeGo-dev@meego.com http://lists.meego.com/listinfo/meego-dev