Some devices with msg2138 have back/menu/home keys.
Add support for them.
Signed-off-by: Vincent Knecht <[email protected]>
---
drivers/input/touchscreen/msg2638.c | 57 +++++++++++++++++++++++++++--
1 file changed, 53 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/msg2638.c b/drivers/input/touchscreen/msg2638.c
index 95b18563326a..a0f5e1ecd612 100644
--- a/drivers/input/touchscreen/msg2638.c
+++ b/drivers/input/touchscreen/msg2638.c
@@ -21,6 +21,7 @@
#include <linux/kernel.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
+#include <linux/property.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
@@ -29,6 +30,8 @@
#define MSG2138_MAX_FINGERS 2
#define MSG2638_MAX_FINGERS 5
+#define MAX_BUTTONS 4
+
#define CHIP_ON_DELAY_MS 15
#define FIRMWARE_ON_DELAY_MS 50
#define RESET_DELAY_MIN_US 10000
@@ -72,6 +75,8 @@ struct msg2638_ts_data {
struct regulator_bulk_data supplies[2];
struct gpio_desc *reset_gpiod;
int max_fingers;
+ u32 keycodes[MAX_BUTTONS];
+ int num_keycodes;
};
static u8 msg2638_checksum(u8 *data, u32 length)
@@ -85,6 +90,18 @@ static u8 msg2638_checksum(u8 *data, u32 length)
return (u8)((-sum) & 0xFF);
}
+static void msg2138_report_keys(struct msg2638_ts_data *msg2638, u8 keys)
+{
+ int i;
+
+ /* keys can be 0x00 or 0xff when all keys have been released */
+ if (keys == 0xff)
+ keys = 0;
+
+ for (i = 0; i < msg2638->num_keycodes; ++i)
+ input_report_key(msg2638->input_dev, msg2638->keycodes[i], keys & BIT(i));
+}
+
static irqreturn_t msg2138_ts_irq_handler(int irq, void *msg2638_handler)
{
struct msg2638_ts_data *msg2638 = msg2638_handler;
@@ -121,9 +138,12 @@ static irqreturn_t msg2138_ts_irq_handler(int irq, void *msg2638_handler)
p0 = &touch_event.pkt[0];
p1 = &touch_event.pkt[1];
- /* Ignore non-pressed finger data */
- if (p0->xy_hi == 0xFF && p0->x_low == 0xFF && p0->y_low == 0xFF)
+ /* Ignore non-pressed finger data, but check for key code */
+ if (p0->xy_hi == 0xFF && p0->x_low == 0xFF && p0->y_low == 0xFF) {
+ if (p1->xy_hi == 0xFF && p1->y_low == 0xFF)
+ msg2138_report_keys(msg2638, p1->x_low);
goto report;
+ }
x = ((p0->xy_hi & 0xF0) << 4) | p0->x_low;
y = ((p0->xy_hi & 0x0F) << 8) | p0->y_low;
@@ -283,6 +303,7 @@ static int msg2638_init_input_dev(struct msg2638_ts_data *msg2638)
struct device *dev = &msg2638->client->dev;
struct input_dev *input_dev;
int error;
+ int i;
input_dev = devm_input_allocate_device(dev);
if (!input_dev) {
@@ -299,6 +320,14 @@ static int msg2638_init_input_dev(struct msg2638_ts_data *msg2638)
input_dev->open = msg2638_input_open;
input_dev->close = msg2638_input_close;
+ if (msg2638->num_keycodes) {
+ input_dev->keycode = msg2638->keycodes;
+ input_dev->keycodemax = msg2638->num_keycodes;
+ input_dev->keycodesize = sizeof(msg2638->keycodes[0]);
+ for (i = 0; i < msg2638->num_keycodes; i++)
+ input_set_capability(input_dev, EV_KEY, msg2638->keycodes[i]);
+ }
+
input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
@@ -367,9 +396,23 @@ static int msg2638_ts_probe(struct i2c_client *client)
return error;
}
- error = msg2638_init_input_dev(msg2638);
+ msg2638->num_keycodes = fwnode_property_count_u32(dev->fwnode, "linux,keycodes");
+ if (msg2638->num_keycodes == -EINVAL) {
+ msg2638->num_keycodes = 0;
+ } else if (msg2638->num_keycodes < 0) {
+ dev_err(dev, "Unable to parse linux,keycodes property: %d\n",
+ msg2638->num_keycodes);
+ return msg2638->num_keycodes;
+ } else if (msg2638->num_keycodes > ARRAY_SIZE(msg2638->keycodes)) {
+ dev_warn(dev, "Found %d linux,keycodes but max is %ld, ignoring the rest\n",
+ msg2638->num_keycodes, ARRAY_SIZE(msg2638->keycodes));
+ msg2638->num_keycodes = ARRAY_SIZE(msg2638->keycodes);
+ }
+
+ error = fwnode_property_read_u32_array(dev->fwnode, "linux,keycodes",
+ msg2638->keycodes, msg2638->num_keycodes);
if (error) {
- dev_err(dev, "Failed to initialize input device: %d\n", error);
+ dev_err(dev, "Unable to read linux,keycodes values: %d\n", error);
return error;
}
@@ -382,6 +425,12 @@ static int msg2638_ts_probe(struct i2c_client *client)
return error;
}
+ error = msg2638_init_input_dev(msg2638);
+ if (error) {
+ dev_err(dev, "Failed to initialize input device: %d\n", error);
+ return error;
+ }
+
return 0;
}
--
2.38.1
Hi Vincent,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on dtor-input/next]
[also build test WARNING on next-20221116]
[cannot apply to dtor-input/for-linus robh/for-next krzk-dt/for-next linus/master v6.1-rc5]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Vincent-Knecht/Input-msg2638-Add-support-for-msg2138-and-key-events/20221117-021842
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20221116181715.2118436-3-vincent.knecht%40mailoo.org
patch subject: [PATCH v5 2/2] Input: msg2638 - Add support for msg2138 key events
config: m68k-allyesconfig
compiler: m68k-linux-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/290da623ffac857edd71f373a1c152e0fd71f593
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Vincent-Knecht/Input-msg2638-Add-support-for-msg2138-and-key-events/20221117-021842
git checkout 290da623ffac857edd71f373a1c152e0fd71f593
# 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=m68k SHELL=/bin/bash drivers/input/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All warnings (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from include/linux/acpi.h:15,
from include/linux/i2c.h:13,
from drivers/input/touchscreen/msg2638.c:16:
drivers/input/touchscreen/msg2638.c: In function 'msg2638_ts_probe':
>> drivers/input/touchscreen/msg2638.c:407:31: warning: format '%ld' expects argument of type 'long int', but argument 4 has type 'unsigned int' [-Wformat=]
407 | dev_warn(dev, "Found %d linux,keycodes but max is %ld, ignoring the rest\n",
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/dev_printk.h:110:30: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~
include/linux/dev_printk.h:146:61: note: in expansion of macro 'dev_fmt'
146 | dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~
drivers/input/touchscreen/msg2638.c:407:17: note: in expansion of macro 'dev_warn'
407 | dev_warn(dev, "Found %d linux,keycodes but max is %ld, ignoring the rest\n",
| ^~~~~~~~
drivers/input/touchscreen/msg2638.c:407:69: note: format string is defined here
407 | dev_warn(dev, "Found %d linux,keycodes but max is %ld, ignoring the rest\n",
| ~~^
| |
| long int
| %d
vim +407 drivers/input/touchscreen/msg2638.c
355
356 static int msg2638_ts_probe(struct i2c_client *client)
357 {
358 const struct msg_chip_data *chip_data;
359 struct device *dev = &client->dev;
360 struct msg2638_ts_data *msg2638;
361 int error;
362
363 if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) {
364 dev_err(dev, "Failed to assert adapter's support for plain I2C.\n");
365 return -ENXIO;
366 }
367
368 msg2638 = devm_kzalloc(dev, sizeof(*msg2638), GFP_KERNEL);
369 if (!msg2638)
370 return -ENOMEM;
371
372 msg2638->client = client;
373 i2c_set_clientdata(client, msg2638);
374
375 chip_data = device_get_match_data(&client->dev);
376 if (!chip_data || !chip_data->max_fingers) {
377 dev_err(dev, "Invalid or missing chip data\n");
378 return -EINVAL;
379 }
380
381 msg2638->max_fingers = chip_data->max_fingers;
382
383 msg2638->supplies[0].supply = "vdd";
384 msg2638->supplies[1].supply = "vddio";
385 error = devm_regulator_bulk_get(dev, ARRAY_SIZE(msg2638->supplies),
386 msg2638->supplies);
387 if (error) {
388 dev_err(dev, "Failed to get regulators: %d\n", error);
389 return error;
390 }
391
392 msg2638->reset_gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
393 if (IS_ERR(msg2638->reset_gpiod)) {
394 error = PTR_ERR(msg2638->reset_gpiod);
395 dev_err(dev, "Failed to request reset GPIO: %d\n", error);
396 return error;
397 }
398
399 msg2638->num_keycodes = fwnode_property_count_u32(dev->fwnode, "linux,keycodes");
400 if (msg2638->num_keycodes == -EINVAL) {
401 msg2638->num_keycodes = 0;
402 } else if (msg2638->num_keycodes < 0) {
403 dev_err(dev, "Unable to parse linux,keycodes property: %d\n",
404 msg2638->num_keycodes);
405 return msg2638->num_keycodes;
406 } else if (msg2638->num_keycodes > ARRAY_SIZE(msg2638->keycodes)) {
> 407 dev_warn(dev, "Found %d linux,keycodes but max is %ld, ignoring the rest\n",
408 msg2638->num_keycodes, ARRAY_SIZE(msg2638->keycodes));
409 msg2638->num_keycodes = ARRAY_SIZE(msg2638->keycodes);
410 }
411
412 error = fwnode_property_read_u32_array(dev->fwnode, "linux,keycodes",
413 msg2638->keycodes, msg2638->num_keycodes);
414 if (error) {
415 dev_err(dev, "Unable to read linux,keycodes values: %d\n", error);
416 return error;
417 }
418
419 error = devm_request_threaded_irq(dev, client->irq,
420 NULL, chip_data->irq_handler,
421 IRQF_ONESHOT | IRQF_NO_AUTOEN,
422 client->name, msg2638);
423 if (error) {
424 dev_err(dev, "Failed to request IRQ: %d\n", error);
425 return error;
426 }
427
428 error = msg2638_init_input_dev(msg2638);
429 if (error) {
430 dev_err(dev, "Failed to initialize input device: %d\n", error);
431 return error;
432 }
433
434 return 0;
435 }
436
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Wed, Nov 16, 2022 at 07:17:12PM +0100, Vincent Knecht wrote:
> Some devices with msg2138 have back/menu/home keys.
> Add support for them.
>
> Signed-off-by: Vincent Knecht <[email protected]>
> ---
> drivers/input/touchscreen/msg2638.c | 57 +++++++++++++++++++++++++++--
> 1 file changed, 53 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/touchscreen/msg2638.c b/drivers/input/touchscreen/msg2638.c
> index 95b18563326a..a0f5e1ecd612 100644
> --- a/drivers/input/touchscreen/msg2638.c
> +++ b/drivers/input/touchscreen/msg2638.c
> @@ -21,6 +21,7 @@
> #include <linux/kernel.h>
> #include <linux/mod_devicetable.h>
> #include <linux/module.h>
> +#include <linux/property.h>
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
>
> @@ -29,6 +30,8 @@
> #define MSG2138_MAX_FINGERS 2
> #define MSG2638_MAX_FINGERS 5
>
> +#define MAX_BUTTONS 4
> +
> #define CHIP_ON_DELAY_MS 15
> #define FIRMWARE_ON_DELAY_MS 50
> #define RESET_DELAY_MIN_US 10000
> @@ -72,6 +75,8 @@ struct msg2638_ts_data {
> struct regulator_bulk_data supplies[2];
> struct gpio_desc *reset_gpiod;
> int max_fingers;
> + u32 keycodes[MAX_BUTTONS];
> + int num_keycodes;
> };
>
> static u8 msg2638_checksum(u8 *data, u32 length)
> @@ -85,6 +90,18 @@ static u8 msg2638_checksum(u8 *data, u32 length)
> return (u8)((-sum) & 0xFF);
> }
>
> +static void msg2138_report_keys(struct msg2638_ts_data *msg2638, u8 keys)
> +{
> + int i;
> +
> + /* keys can be 0x00 or 0xff when all keys have been released */
> + if (keys == 0xff)
> + keys = 0;
> +
> + for (i = 0; i < msg2638->num_keycodes; ++i)
> + input_report_key(msg2638->input_dev, msg2638->keycodes[i], keys & BIT(i));
> +}
> +
> static irqreturn_t msg2138_ts_irq_handler(int irq, void *msg2638_handler)
> {
> struct msg2638_ts_data *msg2638 = msg2638_handler;
> @@ -121,9 +138,12 @@ static irqreturn_t msg2138_ts_irq_handler(int irq, void *msg2638_handler)
> p0 = &touch_event.pkt[0];
> p1 = &touch_event.pkt[1];
>
> - /* Ignore non-pressed finger data */
> - if (p0->xy_hi == 0xFF && p0->x_low == 0xFF && p0->y_low == 0xFF)
> + /* Ignore non-pressed finger data, but check for key code */
> + if (p0->xy_hi == 0xFF && p0->x_low == 0xFF && p0->y_low == 0xFF) {
> + if (p1->xy_hi == 0xFF && p1->y_low == 0xFF)
> + msg2138_report_keys(msg2638, p1->x_low);
> goto report;
> + }
>
> x = ((p0->xy_hi & 0xF0) << 4) | p0->x_low;
> y = ((p0->xy_hi & 0x0F) << 8) | p0->y_low;
> @@ -283,6 +303,7 @@ static int msg2638_init_input_dev(struct msg2638_ts_data *msg2638)
> struct device *dev = &msg2638->client->dev;
> struct input_dev *input_dev;
> int error;
> + int i;
>
> input_dev = devm_input_allocate_device(dev);
> if (!input_dev) {
> @@ -299,6 +320,14 @@ static int msg2638_init_input_dev(struct msg2638_ts_data *msg2638)
> input_dev->open = msg2638_input_open;
> input_dev->close = msg2638_input_close;
>
> + if (msg2638->num_keycodes) {
> + input_dev->keycode = msg2638->keycodes;
> + input_dev->keycodemax = msg2638->num_keycodes;
> + input_dev->keycodesize = sizeof(msg2638->keycodes[0]);
> + for (i = 0; i < msg2638->num_keycodes; i++)
> + input_set_capability(input_dev, EV_KEY, msg2638->keycodes[i]);
> + }
> +
> input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
> input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
>
> @@ -367,9 +396,23 @@ static int msg2638_ts_probe(struct i2c_client *client)
> return error;
> }
>
> - error = msg2638_init_input_dev(msg2638);
> + msg2638->num_keycodes = fwnode_property_count_u32(dev->fwnode, "linux,keycodes");
Please use device_property_count_u32().
> + if (msg2638->num_keycodes == -EINVAL) {
> + msg2638->num_keycodes = 0;
> + } else if (msg2638->num_keycodes < 0) {
> + dev_err(dev, "Unable to parse linux,keycodes property: %d\n",
> + msg2638->num_keycodes);
> + return msg2638->num_keycodes;
> + } else if (msg2638->num_keycodes > ARRAY_SIZE(msg2638->keycodes)) {
> + dev_warn(dev, "Found %d linux,keycodes but max is %ld, ignoring the rest\n",
> + msg2638->num_keycodes, ARRAY_SIZE(msg2638->keycodes));
I think you want "%zd" in place of "%ld".
> + msg2638->num_keycodes = ARRAY_SIZE(msg2638->keycodes);
> + }
> +
> + error = fwnode_property_read_u32_array(dev->fwnode, "linux,keycodes",
> + msg2638->keycodes, msg2638->num_keycodes);
Please use device_property_read_u32_array().
> if (error) {
> - dev_err(dev, "Failed to initialize input device: %d\n", error);
> + dev_err(dev, "Unable to read linux,keycodes values: %d\n", error);
> return error;
> }
>
> @@ -382,6 +425,12 @@ static int msg2638_ts_probe(struct i2c_client *client)
> return error;
> }
>
> + error = msg2638_init_input_dev(msg2638);
> + if (error) {
> + dev_err(dev, "Failed to initialize input device: %d\n", error);
> + return error;
> + }
> +
> return 0;
> }
>
> --
> 2.38.1
>
>
>
Thanks.
--
Dmitry