| Message ID | 20230510-feature-ts_virtobj_patch-v1-3-5ae5e81bc264@wolfvision.net |
|---|---|
| State | New |
| Headers | show |
| Series | Input: support virtual objects on touchscreens | expand |
Hi Javier,
kernel test robot noticed the following build errors:
[auto build test ERROR on ac9a78681b921877518763ba0e89202254349d1b]
url: https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519
base: ac9a78681b921877518763ba0e89202254349d1b
patch link: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v1-3-5ae5e81bc264%40wolfvision.net
patch subject: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling
config: arm-randconfig-m041-20230514 (https://download.01.org/0day-ci/archive/20230514/202305140640.VLcvhR5G-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/133c0f8c33dc5e70a72e6a7d670e133b6043a7a3
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519
git checkout 133c0f8c33dc5e70a72e6a7d670e133b6043a7a3
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202305140640.VLcvhR5G-lkp@intel.com/
All errors (new ones prefixed by >>):
arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_parse_and_report':
>> st1232.c:(.text+0x148): undefined reference to `ts_virtobj_is_button_slot'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x15e): undefined reference to `ts_virtobj_button_press'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x16c): undefined reference to `ts_virtobj_mt_on_touchscreen'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x242): undefined reference to `ts_virtobj_mapped_buttons'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x266): undefined reference to `ts_virtobj_is_button_slot'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x276): undefined reference to `ts_virtobj_button_release'
arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_probe':
>> st1232.c:(.text+0x42c): undefined reference to `ts_virtobj_map_objects'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x43c): undefined reference to `ts_virtobj_mapped_touchscreen'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x44a): undefined reference to `ts_virtobj_get_touchscreen_abs'
arm-linux-gnueabi-ld: st1232.c:(.text+0x520): undefined reference to `ts_virtobj_mapped_buttons'
>> arm-linux-gnueabi-ld: st1232.c:(.text+0x542): undefined reference to `ts_virtobj_set_button_caps'
On 15/05/2023 06:34, Javier Carrasco wrote: > On 14.05.23 00:38, kernel test robot wrote: >> Hi Javier, >> >> kernel test robot noticed the following build errors: >> >> [auto build test ERROR on ac9a78681b921877518763ba0e89202254349d1b] >> >> url: https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519 >> base: ac9a78681b921877518763ba0e89202254349d1b >> patch link: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v1-3-5ae5e81bc264%40wolfvision.net >> patch subject: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling >> config: arm-randconfig-m041-20230514 (https://download.01.org/0day-ci/archive/20230514/202305140640.VLcvhR5G-lkp@intel.com/config) >> compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0 >> reproduce (this is a W=1 build): >> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross >> chmod +x ~/bin/make.cross >> # https://github.com/intel-lab-lkp/linux/commit/133c0f8c33dc5e70a72e6a7d670e133b6043a7a3 >> git remote add linux-review https://github.com/intel-lab-lkp/linux >> git fetch --no-tags linux-review Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519 >> git checkout 133c0f8c33dc5e70a72e6a7d670e133b6043a7a3 >> # save the config file >> mkdir build_dir && cp config build_dir/.config >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig >> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash >> >> If you fix the issue, kindly add following tag where applicable >> | Reported-by: kernel test robot <lkp@intel.com> >> | Link: https://lore.kernel.org/oe-kbuild-all/202305140640.VLcvhR5G-lkp@intel.com/ >> >> All errors (new ones prefixed by >>): >> >> arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_parse_and_report': >>>> st1232.c:(.text+0x148): undefined reference to `ts_virtobj_is_button_slot' >>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x15e): undefined reference to `ts_virtobj_button_press' >>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x16c): undefined reference to `ts_virtobj_mt_on_touchscreen' >>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x242): undefined reference to `ts_virtobj_mapped_buttons' >>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x266): undefined reference to `ts_virtobj_is_button_slot' >>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x276): undefined reference to `ts_virtobj_button_release' >> arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_probe': >>>> st1232.c:(.text+0x42c): undefined reference to `ts_virtobj_map_objects' >>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x43c): undefined reference to `ts_virtobj_mapped_touchscreen' >>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x44a): undefined reference to `ts_virtobj_get_touchscreen_abs' >> arm-linux-gnueabi-ld: st1232.c:(.text+0x520): undefined reference to `ts_virtobj_mapped_buttons' >>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x542): undefined reference to `ts_virtobj_set_button_caps' >> > Apparently there is something wrong about the references from this patch > to a previous one from the same series ([PATCH 1/4] Input: ts-virtobj - > Add touchs[c]reen virtual object handling). The "url" link shows all > patches of this series in the right order though. > > All these functions are declared in the linux/input/ts-virtobj.h header > and also inline-defined there if ts-virtobj is not selected. If it is > selected (either y or M), the functions are exported from > driver/input/touchscreen/ts-virtobj.c. According to the error report, > ts-virtobj was selected as a module. > > I could build the kernel with the three possible configurations > (ts-virtobj y/n/M) for x86_64 as well as for arm64 with no errors or > warnings repeatedly, so I am a bit confused now. I am probably > missing something, but I do not know what. > > I wonder if the new file where the functions are defined (ts-virtobj.c) > could not be found by some reason or if the test build does not like the > way I handled the function declaration/definition. Any hint or advice > would be more than welcome. The report is correct. Build driver builtin and your virtual as module. Best regards, Krzysztof
On 15.05.23 08:43, Krzysztof Kozlowski wrote: > On 15/05/2023 06:34, Javier Carrasco wrote: >> On 14.05.23 00:38, kernel test robot wrote: >>> Hi Javier, >>> >>> kernel test robot noticed the following build errors: >>> >>> [auto build test ERROR on ac9a78681b921877518763ba0e89202254349d1b] >>> >>> url: https://github.com/intel-lab-lkp/linux/commits/Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519 >>> base: ac9a78681b921877518763ba0e89202254349d1b >>> patch link: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v1-3-5ae5e81bc264%40wolfvision.net >>> patch subject: [PATCH 3/4] Input: st1232 - add virtual touchscreen and buttons handling >>> config: arm-randconfig-m041-20230514 (https://download.01.org/0day-ci/archive/20230514/202305140640.VLcvhR5G-lkp@intel.com/config) >>> compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0 >>> reproduce (this is a W=1 build): >>> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross >>> chmod +x ~/bin/make.cross >>> # https://github.com/intel-lab-lkp/linux/commit/133c0f8c33dc5e70a72e6a7d670e133b6043a7a3 >>> git remote add linux-review https://github.com/intel-lab-lkp/linux >>> git fetch --no-tags linux-review Javier-Carrasco/Input-ts-virtobj-Add-touchsreen-virtual-object-handling/20230510-215519 >>> git checkout 133c0f8c33dc5e70a72e6a7d670e133b6043a7a3 >>> # save the config file >>> mkdir build_dir && cp config build_dir/.config >>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig >>> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash >>> >>> If you fix the issue, kindly add following tag where applicable >>> | Reported-by: kernel test robot <lkp@intel.com> >>> | Link: https://lore.kernel.org/oe-kbuild-all/202305140640.VLcvhR5G-lkp@intel.com/ >>> >>> All errors (new ones prefixed by >>): >>> >>> arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_parse_and_report': >>>>> st1232.c:(.text+0x148): undefined reference to `ts_virtobj_is_button_slot' >>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x15e): undefined reference to `ts_virtobj_button_press' >>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x16c): undefined reference to `ts_virtobj_mt_on_touchscreen' >>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x242): undefined reference to `ts_virtobj_mapped_buttons' >>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x266): undefined reference to `ts_virtobj_is_button_slot' >>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x276): undefined reference to `ts_virtobj_button_release' >>> arm-linux-gnueabi-ld: drivers/input/touchscreen/st1232.o: in function `st1232_ts_probe': >>>>> st1232.c:(.text+0x42c): undefined reference to `ts_virtobj_map_objects' >>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x43c): undefined reference to `ts_virtobj_mapped_touchscreen' >>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x44a): undefined reference to `ts_virtobj_get_touchscreen_abs' >>> arm-linux-gnueabi-ld: st1232.c:(.text+0x520): undefined reference to `ts_virtobj_mapped_buttons' >>>>> arm-linux-gnueabi-ld: st1232.c:(.text+0x542): undefined reference to `ts_virtobj_set_button_caps' >>> >> Apparently there is something wrong about the references from this patch >> to a previous one from the same series ([PATCH 1/4] Input: ts-virtobj - >> Add touchs[c]reen virtual object handling). The "url" link shows all >> patches of this series in the right order though. >> >> All these functions are declared in the linux/input/ts-virtobj.h header >> and also inline-defined there if ts-virtobj is not selected. If it is >> selected (either y or M), the functions are exported from >> driver/input/touchscreen/ts-virtobj.c. According to the error report, >> ts-virtobj was selected as a module. >> >> I could build the kernel with the three possible configurations >> (ts-virtobj y/n/M) for x86_64 as well as for arm64 with no errors or >> warnings repeatedly, so I am a bit confused now. I am probably >> missing something, but I do not know what. >> >> I wonder if the new file where the functions are defined (ts-virtobj.c) >> could not be found by some reason or if the test build does not like the >> way I handled the function declaration/definition. Any hint or advice >> would be more than welcome. > > The report is correct. Build driver builtin and your virtual as module. You are right, that was the configuration I was missing, which I honestly did not even consider when I tested this feature. The problem seems to be that I am using the IS_ENABLED macro which returns true if selected as a module, but the define is in that case CONFIG_TOUCHSCREEN_TS_VIRTOBJ_MODULE instead of CONFIG_TOUCHSCREEN_TS_VIRTOBJ. As I am using the latter define in the Makefile, it does not get compiled. I am testing the IS_REACHABLE macro for the header instead, which I think is the right one in this case and actually fixes the bug. I am telling you that just in case I am talking nonsense to save everyone from a pointless v2. But first I will go through all possible combinations between my virtual and a touchscreen driver before I submit a broken v2, where I will also provide an improved documentation as you suggested. Thanks a lot again! > > Best regards, > Krzysztof >
On 15.05.23 10:41, Krzysztof Kozlowski wrote: > On 15/05/2023 10:33, Javier Carrasco wrote: >>>> All these functions are declared in the linux/input/ts-virtobj.h header >>>> and also inline-defined there if ts-virtobj is not selected. If it is >>>> selected (either y or M), the functions are exported from >>>> driver/input/touchscreen/ts-virtobj.c. According to the error report, >>>> ts-virtobj was selected as a module. >>>> >>>> I could build the kernel with the three possible configurations >>>> (ts-virtobj y/n/M) for x86_64 as well as for arm64 with no errors or >>>> warnings repeatedly, so I am a bit confused now. I am probably >>>> missing something, but I do not know what. >>>> >>>> I wonder if the new file where the functions are defined (ts-virtobj.c) >>>> could not be found by some reason or if the test build does not like the >>>> way I handled the function declaration/definition. Any hint or advice >>>> would be more than welcome. >>> >>> The report is correct. Build driver builtin and your virtual as module. >> >> You are right, that was the configuration I was missing, which I >> honestly did not even consider when I tested this feature. >> >> The problem seems to be that I am using the IS_ENABLED macro which >> returns true if selected as a module, but the define is in that case >> CONFIG_TOUCHSCREEN_TS_VIRTOBJ_MODULE instead of >> CONFIG_TOUCHSCREEN_TS_VIRTOBJ. As I am using the latter define in the >> Makefile, it does not get compiled. > > This could be proper solution for build failure but does not look like > the proper solution for entire choice/design. The questions are: why > TOUCHSCREEN_TS_VIRTOBJ should be selectable by user? How can user know > that he needs a driver with VIRTOBJ? > > I think he cannot and your config help option suggests that: > "you are using a touchscreen driver that supports printed overlays". > What driver supports or not, is a job for kernel developers. Not for > kernel configurators or distros. User should never investigate whether > his touchscreen driver support printed overlays. Why would user like to > dig into kernel to know that? Thus either your driver should select > VIRTOBJ or depend on it. > I was thinking about the minimal savings (inline functions that just return a value and the size of the ts-virtobj object itself) that could be obtained if the touchscreen does not need the functionality at all. But probably those savings are negligible and there will not be many cases where the user will know if that applies. I agree, the feature will be selected by drivers that support it. That actually simplifies the code a little bit. > Best regards, > Krzysztof >
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c index f49566dc96f8..b8139b7daa40 100644 --- a/drivers/input/touchscreen/st1232.c +++ b/drivers/input/touchscreen/st1232.c @@ -22,6 +22,7 @@ #include <linux/pm_qos.h> #include <linux/slab.h> #include <linux/types.h> +#include <linux/input/ts-virtobj.h> #define ST1232_TS_NAME "st1232-ts" #define ST1633_TS_NAME "st1633-ts" @@ -56,6 +57,8 @@ struct st1232_ts_data { struct touchscreen_properties prop; struct dev_pm_qos_request low_latency_req; struct gpio_desc *reset_gpio; + struct input_dev *virtual_keypad; + struct ts_virtobj_map *map; const struct st_chip_info *chip_info; int read_buf_len; u8 *read_buf; @@ -129,10 +132,12 @@ static int st1232_ts_read_resolution(struct st1232_ts_data *ts, u16 *max_x, static int st1232_ts_parse_and_report(struct st1232_ts_data *ts) { - struct input_dev *input = ts->input_dev; + struct input_dev *tscreen = ts->input_dev; + __maybe_unused struct input_dev *keypad = ts->virtual_keypad; struct input_mt_pos pos[ST_TS_MAX_FINGERS]; u8 z[ST_TS_MAX_FINGERS]; int slots[ST_TS_MAX_FINGERS]; + __maybe_unused bool button_pressed[ST_TS_MAX_FINGERS]; int n_contacts = 0; int i; @@ -143,6 +148,15 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts) unsigned int x = ((buf[0] & 0x70) << 4) | buf[1]; unsigned int y = ((buf[0] & 0x07) << 8) | buf[2]; + /* forward button presses to the keypad input device */ + if (ts_virtobj_is_button_slot(ts->map, i) || + ts_virtobj_button_press(ts->map, keypad, x, y, i)) { + button_pressed[i] = true; + continue; + } + /* Ignore events out of the virtual screen if defined */ + if (!ts_virtobj_mt_on_touchscreen(ts->map, &x, &y)) + continue; touchscreen_set_mt_pos(&pos[n_contacts], &ts->prop, x, y); @@ -154,18 +168,25 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts) } } - input_mt_assign_slots(input, slots, pos, n_contacts, 0); + input_mt_assign_slots(tscreen, slots, pos, n_contacts, 0); for (i = 0; i < n_contacts; i++) { - input_mt_slot(input, slots[i]); - input_mt_report_slot_state(input, MT_TOOL_FINGER, true); - input_report_abs(input, ABS_MT_POSITION_X, pos[i].x); - input_report_abs(input, ABS_MT_POSITION_Y, pos[i].y); + input_mt_slot(tscreen, slots[i]); + input_mt_report_slot_state(tscreen, MT_TOOL_FINGER, true); + input_report_abs(tscreen, ABS_MT_POSITION_X, pos[i].x); + input_report_abs(tscreen, ABS_MT_POSITION_Y, pos[i].y); if (ts->chip_info->have_z) - input_report_abs(input, ABS_MT_TOUCH_MAJOR, z[i]); + input_report_abs(tscreen, ABS_MT_TOUCH_MAJOR, z[i]); + } + input_mt_sync_frame(tscreen); + input_sync(tscreen); + + if (ts_virtobj_mapped_buttons(ts->map)) { + for (i = 0; i < ts->chip_info->max_fingers; i++) + if (ts_virtobj_is_button_slot(ts->map, i) && + !button_pressed[i]) + ts_virtobj_button_release(ts->map, keypad, i); + input_sync(keypad); } - - input_mt_sync_frame(input); - input_sync(input); return n_contacts; } @@ -226,6 +247,7 @@ static int st1232_ts_probe(struct i2c_client *client) const struct st_chip_info *match; struct st1232_ts_data *ts; struct input_dev *input_dev; + struct input_dev __maybe_unused *virtual_keypad; u16 max_x, max_y; int error; @@ -292,18 +314,28 @@ static int st1232_ts_probe(struct i2c_client *client) if (error) return error; - /* Read resolution from the chip */ - error = st1232_ts_read_resolution(ts, &max_x, &max_y); - if (error) { - dev_err(&client->dev, - "Failed to read resolution: %d\n", error); - return error; - } - if (ts->chip_info->have_z) input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0, ts->chip_info->max_area, 0, 0); + /* map virtual objects if defined in the device tree */ + ts->map = ts_virtobj_map_objects(&ts->client->dev, ts->input_dev); + if (IS_ERR(ts->map)) + return PTR_ERR(ts->map); + + if (ts_virtobj_mapped_touchscreen(ts->map)) { + /* Read resolution from the virtual touchscreen if defined*/ + ts_virtobj_get_touchscreen_abs(ts->map, &max_x, &max_y); + } else { + /* Read resolution from the chip */ + error = st1232_ts_read_resolution(ts, &max_x, &max_y); + if (error) { + dev_err(&client->dev, + "Failed to read resolution: %d\n", error); + return error; + } + } + input_set_abs_params(input_dev, ABS_MT_POSITION_X, 0, max_x, 0, 0); input_set_abs_params(input_dev, ABS_MT_POSITION_Y, @@ -335,6 +367,25 @@ static int st1232_ts_probe(struct i2c_client *client) return error; } + /* Register keypad input device if virtual buttons were defined */ + if (ts_virtobj_mapped_buttons(ts->map)) { + virtual_keypad = devm_input_allocate_device(&client->dev); + if (!virtual_keypad) + return -ENOMEM; + + ts->virtual_keypad = virtual_keypad; + virtual_keypad->name = "st1232-keypad"; + virtual_keypad->id.bustype = BUS_I2C; + ts_virtobj_set_button_caps(ts->map, virtual_keypad); + error = input_register_device(ts->virtual_keypad); + if (error) { + dev_err(&client->dev, + "Unable to register %s input device\n", + virtual_keypad->name); + return error; + } + } + i2c_set_clientdata(client, ts); return 0;
Use ts-virtobj to support overlay objects such as buttons and resized frames defined in the device tree. Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net> --- drivers/input/touchscreen/st1232.c | 87 ++++++++++++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 18 deletions(-)