[PATCH 1/6] Input: mms114 - fix touch indexing for MMS134S and MMS136

[PATCH 1/6] Input: mms114 - fix touch indexing for MMS134S and MMS136

[Dmitry Torokhov]

The MMS134S and MMS136 touch controllers have an event size of 6 bytes
rather than 8 bytes. When __mms114_read_reg() reads the touch data
packet from the device into the touch buffer, the events are packed
tightly at 6-byte intervals. However, the driver iterates through the
events using standard C array indexing (touch[index]), where each
element is sizeof(struct mms114_touch) (8 bytes) apart. As a result, any
touch events beyond the first one are read from incorrect offsets and
parsed improperly.

Fix this by explicitly calculating the byte offset for each touch event
based on the device's specific event size.

Fixes: ab108678195f ("Input: mms114 - support MMS134S")
Reported-by: sashiko-bot@kernel.org
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/mms114.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index 4d748a13408d..53ad35d61d47 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -217,7 +217,9 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
 	struct mms114_data *data = dev_id;
 	struct i2c_client *client = data->client;
 	struct mms114_touch touch[MMS114_MAX_TOUCH];
+	struct mms114_touch *t;
 	int packet_size;
+	int event_size;
 	int touch_size;
 	int index;
 	int error;
@@ -234,9 +236,11 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
 
 	/* MMS136 has slightly different event size */
 	if (data->type == TYPE_MMS134S || data->type == TYPE_MMS136)
-		touch_size = packet_size / MMS136_EVENT_SIZE;
+		event_size = MMS136_EVENT_SIZE;
 	else
-		touch_size = packet_size / MMS114_EVENT_SIZE;
+		event_size = MMS114_EVENT_SIZE;
+
+	touch_size = packet_size / event_size;
 
 	error = __mms114_read_reg(data, MMS114_INFORMATION, packet_size,
 			(u8 *)touch);
@@ -244,18 +248,20 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
 		goto out;
 
 	for (index = 0; index < touch_size; index++) {
-		switch (touch[index].type) {
+		t = (struct mms114_touch *)((u8 *)touch + index * event_size);
+
+		switch (t->type) {
 		case MMS114_TYPE_TOUCHSCREEN:
-			mms114_process_mt(data, touch + index);
+			mms114_process_mt(data, t);
 			break;
 
 		case MMS114_TYPE_TOUCHKEY:
-			mms114_process_touchkey(data, touch + index);
+			mms114_process_touchkey(data, t);
 			break;
 
 		default:
 			dev_err(&client->dev, "Wrong touch type (%d)\n",
-				touch[index].type);
+				t->type);
 			break;
 		}
 	}
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 2/6] Input: mms114 - prefer GPL over GPL v2 for module license

[Dmitry Torokhov]

As explained in commit bf7fbeeae6db ("module: Cure the MODULE_LICENSE
"GPL" vs. "GPL v2" bogosity"), "GPL" and "GPL v2" have identical
semantics in the module loader, but "GPL" is preferred to avoid
unnecessary confusion and maintain consistency across the kernel.

Change MODULE_LICENSE("GPL v2") to MODULE_LICENSE("GPL").

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/mms114.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index 53ad35d61d47..db23b51f4630 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -722,4 +722,4 @@ module_i2c_driver(mms114_driver);
 /* Module information */
 MODULE_AUTHOR("Joonyoung Shim <jy0922.shim@samsung.com>");
 MODULE_DESCRIPTION("MELFAS mms114 Touchscreen driver");
-MODULE_LICENSE("GPL v2");
+MODULE_LICENSE("GPL");
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 3/6] Input: mms114 - use appropriate register argument types

[Dmitry Torokhov]

The MMS114 I2C touch controller uses 8-bit register addresses (0x01 to
0xF2) and 8-bit single-register data values. The helper functions
previously declared reg and val as 32-bit unsigned int, requiring
explicit bitwise masking (& 0xff) to narrow the values down to u8 before
populating the I2C transfer buffers.

Update reg and val parameters to u8 across mms114_read_reg(),
mms114_write_reg(), and __mms114_read_reg() to accurately reflect the
hardware specification and eliminate the redundant & 0xff masking.

Additionally, update the val buffer pointer in __mms114_read_reg() from
u8 * to void * to allow callers to pass data structures directly without
requiring explicit casting.

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/mms114.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index db23b51f4630..c2e006ac1196 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -87,12 +87,12 @@ struct mms114_touch {
 	u8 reserved[2];
 } __packed;
 
-static int __mms114_read_reg(struct mms114_data *data, unsigned int reg,
-			     unsigned int len, u8 *val)
+static int __mms114_read_reg(struct mms114_data *data, u8 reg,
+			     unsigned int len, void *val)
 {
 	struct i2c_client *client = data->client;
 	struct i2c_msg xfer[2];
-	u8 buf = reg & 0xff;
+	u8 buf = reg;
 	int error;
 
 	if (reg <= MMS114_MODE_CONTROL && reg + len > MMS114_MODE_CONTROL)
@@ -121,7 +121,7 @@ static int __mms114_read_reg(struct mms114_data *data, unsigned int reg,
 	return 0;
 }
 
-static int mms114_read_reg(struct mms114_data *data, unsigned int reg)
+static int mms114_read_reg(struct mms114_data *data, u8 reg)
 {
 	u8 val;
 	int error;
@@ -133,15 +133,14 @@ static int mms114_read_reg(struct mms114_data *data, unsigned int reg)
 	return error < 0 ? error : val;
 }
 
-static int mms114_write_reg(struct mms114_data *data, unsigned int reg,
-			    unsigned int val)
+static int mms114_write_reg(struct mms114_data *data, u8 reg, u8 val)
 {
 	struct i2c_client *client = data->client;
 	u8 buf[2];
 	int error;
 
-	buf[0] = reg & 0xff;
-	buf[1] = val & 0xff;
+	buf[0] = reg;
+	buf[1] = val;
 
 	error = i2c_master_send(client, buf, 2);
 	if (error != 2) {
@@ -242,9 +241,8 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
 
 	touch_size = packet_size / event_size;
 
-	error = __mms114_read_reg(data, MMS114_INFORMATION, packet_size,
-			(u8 *)touch);
-	if (error < 0)
+	error = __mms114_read_reg(data, MMS114_INFORMATION, packet_size, touch);
+	if (error)
 		goto out;
 
 	for (index = 0; index < touch_size; index++) {
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 4/6] Input: mms114 - replace udelay with usleep_range

[Dmitry Torokhov]

The driver currently uses udelay(MMS114_I2C_DELAY) (50us) to ensure a
mandatory delay between I2C transfers in __mms114_read_reg() and
mms114_write_reg().

Both functions invoke underlying I2C core operations (i2c_transfer,
i2c_master_send) which acquire mutexes and sleep. Furthermore, the
interrupt handler mms114_interrupt() is registered as a threaded IRQ
handler. Since the entire execution path is fully sleepable,
busy-waiting with udelay() for 50us unnecessarily wastes CPU cycles.

Replace udelay() with usleep_range() to allow the CPU to enter low-power
states or execute other tasks during the delay.

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/mms114.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index c2e006ac1196..c59aec8f2feb 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -116,7 +116,7 @@ static int __mms114_read_reg(struct mms114_data *data, u8 reg,
 			"%s: i2c transfer failed (%d)\n", __func__, error);
 		return error < 0 ? error : -EIO;
 	}
-	udelay(MMS114_I2C_DELAY);
+	usleep_range(MMS114_I2C_DELAY, MMS114_I2C_DELAY + 50);
 
 	return 0;
 }
@@ -148,7 +148,7 @@ static int mms114_write_reg(struct mms114_data *data, u8 reg, u8 val)
 			"%s: i2c send failed (%d)\n", __func__, error);
 		return error < 0 ? error : -EIO;
 	}
-	udelay(MMS114_I2C_DELAY);
+	usleep_range(MMS114_I2C_DELAY, MMS114_I2C_DELAY + 50);
 
 	if (reg == MMS114_MODE_CONTROL)
 		data->cache_mode_control = val;
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 5/6] Input: mms114 - replace BUG() and fix alignment

[Dmitry Torokhov]

Avoid crashing the kernel with BUG() when userspace attempts to read
the write-only mode control register. Instead, trigger a warning with
WARN_ON() and return -EINVAL to allow the kernel to recover.

Additionally, fix parameter alignment to match the open parenthesis
in several functions to conform to the kernel coding style.

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/mms114.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index c59aec8f2feb..bf01eee0560a 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -95,8 +95,8 @@ static int __mms114_read_reg(struct mms114_data *data, u8 reg,
 	u8 buf = reg;
 	int error;
 
-	if (reg <= MMS114_MODE_CONTROL && reg + len > MMS114_MODE_CONTROL)
-		BUG();
+	if (WARN_ON(reg <= MMS114_MODE_CONTROL && reg + len > MMS114_MODE_CONTROL))
+		return -EINVAL;
 
 	/* Write register */
 	xfer[0].addr = client->addr;
@@ -310,8 +310,7 @@ static int mms114_get_version(struct mms114_data *data)
 		if (error)
 			return error;
 
-		group = i2c_smbus_read_byte_data(data->client,
-						  MMS152_COMPAT_GROUP);
+		group = i2c_smbus_read_byte_data(data->client, MMS152_COMPAT_GROUP);
 		if (group < 0)
 			return group;
 
@@ -371,14 +370,14 @@ static int mms114_setup_regs(struct mms114_data *data)
 
 	if (data->contact_threshold) {
 		error = mms114_write_reg(data, MMS114_CONTACT_THRESHOLD,
-				data->contact_threshold);
+					 data->contact_threshold);
 		if (error < 0)
 			return error;
 	}
 
 	if (data->moving_threshold) {
 		error = mms114_write_reg(data, MMS114_MOVING_THRESHOLD,
-				data->moving_threshold);
+					 data->moving_threshold);
 		if (error < 0)
 			return error;
 	}
@@ -464,9 +463,9 @@ static int mms114_parse_legacy_bindings(struct mms114_data *data)
 	}
 
 	device_property_read_u32(dev, "contact-threshold",
-				&data->contact_threshold);
+				 &data->contact_threshold);
 	device_property_read_u32(dev, "moving-threshold",
-				&data->moving_threshold);
+				 &data->moving_threshold);
 
 	if (device_property_read_bool(dev, "x-invert"))
 		props->invert_x = true;
@@ -519,7 +518,7 @@ static int mms114_probe(struct i2c_client *client)
 		return data->num_keycodes;
 	} else if (data->num_keycodes > MMS114_MAX_TOUCHKEYS) {
 		dev_warn(&client->dev,
-			"Found %d linux,keycodes but max is %d, ignoring the rest\n",
+			 "Found %d linux,keycodes but max is %d, ignoring the rest\n",
 			 data->num_keycodes, MMS114_MAX_TOUCHKEYS);
 		data->num_keycodes = MMS114_MAX_TOUCHKEYS;
 	}
-- 
2.54.0.1136.gdb2ca164c4-goog

[PATCH 6/6] Input: mms114 - refactor chip variant handling using descriptors

[Dmitry Torokhov]

Instead of using an enum and conditional switch/if statements throughout
the driver to handle differences between chip variants (MMS114, MMS134S,
MMS136, MMS152, MMS345L), introduce a variant-specific descriptor
structure that encapsulates variant-specific properties (name, event
size, presence of configuration registers) and callbacks (such as
get_version). Define descriptors for each supported chip and associate
them with the matching entries in the OF and I2C device ID tables.

This eliminates the need for variant checks in the driver logic, making
it easier to support new chip variants in the future.

Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/touchscreen/mms114.c | 204 ++++++++++++++++-------------
 1 file changed, 112 insertions(+), 92 deletions(-)

diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
index bf01eee0560a..006dded17eb8 100644
--- a/drivers/input/touchscreen/mms114.c
+++ b/drivers/input/touchscreen/mms114.c
@@ -52,21 +52,13 @@
 #define MMS114_TYPE_TOUCHSCREEN		1
 #define MMS114_TYPE_TOUCHKEY		2
 
-enum mms_type {
-	TYPE_MMS114	= 114,
-	TYPE_MMS134S	= 134,
-	TYPE_MMS136	= 136,
-	TYPE_MMS152	= 152,
-	TYPE_MMS345L	= 345,
-};
-
 struct mms114_data {
+	const struct mms_chip	*chip;
 	struct i2c_client	*client;
 	struct input_dev	*input_dev;
 	struct regulator	*core_reg;
 	struct regulator	*io_reg;
 	struct touchscreen_properties props;
-	enum mms_type		type;
 	unsigned int		contact_threshold;
 	unsigned int		moving_threshold;
 
@@ -77,6 +69,13 @@ struct mms114_data {
 	u8			cache_mode_control;
 };
 
+struct mms_chip {
+	const char *name;
+	int event_size;
+	bool has_config_regs;
+	int (*get_version)(struct mms114_data *data);
+};
+
 struct mms114_touch {
 	u8 id:4, reserved_bit4:1, type:2, pressed:1;
 	u8 x_hi:4, y_hi:4;
@@ -156,6 +155,91 @@ static int mms114_write_reg(struct mms114_data *data, u8 reg, u8 val)
 	return 0;
 }
 
+static int mms114_get_version(struct mms114_data *data)
+{
+	struct device *dev = &data->client->dev;
+	u8 buf[6];
+	int error;
+
+	error = __mms114_read_reg(data, MMS114_TSP_REV, 6, buf);
+	if (error)
+		return error;
+
+	dev_info(dev, "TSP Rev: 0x%x, HW Rev: 0x%x, Firmware Ver: 0x%x\n",
+		 buf[0], buf[1], buf[3]);
+	return 0;
+}
+
+static int mms152_get_version(struct mms114_data *data)
+{
+	struct device *dev = &data->client->dev;
+	u8 buf[3];
+	int group;
+	int error;
+
+	error = __mms114_read_reg(data, MMS152_FW_REV, 3, buf);
+	if (error)
+		return error;
+
+	group = i2c_smbus_read_byte_data(data->client, MMS152_COMPAT_GROUP);
+	if (group < 0)
+		return group;
+
+	dev_info(dev, "TSP FW Rev: bootloader 0x%x / core 0x%x / config 0x%x, Compat group: %c\n",
+		 buf[0], buf[1], buf[2], group);
+	return 0;
+}
+
+static int mms345l_get_version(struct mms114_data *data)
+{
+	struct device *dev = &data->client->dev;
+	u8 buf[3];
+	int error;
+
+	error = __mms114_read_reg(data, MMS152_FW_REV, 3, buf);
+	if (error)
+		return error;
+
+	dev_info(dev, "TSP FW Rev: bootloader 0x%x / core 0x%x / config 0x%x\n",
+		 buf[0], buf[1], buf[2]);
+	return 0;
+}
+
+static const struct mms_chip mms114_descriptor = {
+	.name = "MMS114",
+	.event_size = MMS114_EVENT_SIZE,
+	.has_config_regs = true,
+	.get_version = mms114_get_version,
+};
+
+static const struct mms_chip mms134s_descriptor = {
+	.name = "MMS134S",
+	.event_size = MMS136_EVENT_SIZE,
+	.has_config_regs = true,
+	.get_version = mms114_get_version,
+};
+
+static const struct mms_chip mms136_descriptor = {
+	.name = "MMS136",
+	.event_size = MMS136_EVENT_SIZE,
+	.has_config_regs = true,
+	.get_version = mms114_get_version,
+};
+
+static const struct mms_chip mms152_descriptor = {
+	.name = "MMS152",
+	.event_size = MMS114_EVENT_SIZE,
+	.has_config_regs = false,
+	.get_version = mms152_get_version,
+};
+
+static const struct mms_chip mms345l_descriptor = {
+	.name = "MMS345L",
+	.event_size = MMS114_EVENT_SIZE,
+	.has_config_regs = false,
+	.get_version = mms345l_get_version,
+};
+
 static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *touch)
 {
 	struct i2c_client *client = data->client;
@@ -217,8 +301,8 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
 	struct i2c_client *client = data->client;
 	struct mms114_touch touch[MMS114_MAX_TOUCH];
 	struct mms114_touch *t;
+	int event_size = data->chip->event_size;
 	int packet_size;
-	int event_size;
 	int touch_size;
 	int index;
 	int error;
@@ -233,12 +317,6 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
 		goto out;
 	}
 
-	/* MMS136 has slightly different event size */
-	if (data->type == TYPE_MMS134S || data->type == TYPE_MMS136)
-		event_size = MMS136_EVENT_SIZE;
-	else
-		event_size = MMS114_EVENT_SIZE;
-
 	touch_size = packet_size / event_size;
 
 	error = __mms114_read_reg(data, MMS114_INFORMATION, packet_size, touch);
@@ -288,64 +366,17 @@ static int mms114_set_active(struct mms114_data *data, bool active)
 	return mms114_write_reg(data, MMS114_MODE_CONTROL, val);
 }
 
-static int mms114_get_version(struct mms114_data *data)
-{
-	struct device *dev = &data->client->dev;
-	u8 buf[6];
-	int group;
-	int error;
-
-	switch (data->type) {
-	case TYPE_MMS345L:
-		error = __mms114_read_reg(data, MMS152_FW_REV, 3, buf);
-		if (error)
-			return error;
-
-		dev_info(dev, "TSP FW Rev: bootloader 0x%x / core 0x%x / config 0x%x\n",
-			 buf[0], buf[1], buf[2]);
-		break;
-
-	case TYPE_MMS152:
-		error = __mms114_read_reg(data, MMS152_FW_REV, 3, buf);
-		if (error)
-			return error;
-
-		group = i2c_smbus_read_byte_data(data->client, MMS152_COMPAT_GROUP);
-		if (group < 0)
-			return group;
-
-		dev_info(dev, "TSP FW Rev: bootloader 0x%x / core 0x%x / config 0x%x, Compat group: %c\n",
-			 buf[0], buf[1], buf[2], group);
-		break;
-
-	case TYPE_MMS114:
-	case TYPE_MMS134S:
-	case TYPE_MMS136:
-		error = __mms114_read_reg(data, MMS114_TSP_REV, 6, buf);
-		if (error)
-			return error;
-
-		dev_info(dev, "TSP Rev: 0x%x, HW Rev: 0x%x, Firmware Ver: 0x%x\n",
-			 buf[0], buf[1], buf[3]);
-		break;
-	}
-
-	return 0;
-}
-
 static int mms114_setup_regs(struct mms114_data *data)
 {
 	const struct touchscreen_properties *props = &data->props;
 	int val;
 	int error;
 
-	error = mms114_get_version(data);
-	if (error < 0)
+	error = data->chip->get_version(data);
+	if (error)
 		return error;
 
-	/* MMS114, MMS134S and MMS136 have configuration and power on registers */
-	if (data->type != TYPE_MMS114 && data->type != TYPE_MMS134S &&
-	    data->type != TYPE_MMS136)
+	if (!data->chip->has_config_regs)
 		return 0;
 
 	error = mms114_set_active(data, true);
@@ -481,7 +512,6 @@ static int mms114_probe(struct i2c_client *client)
 {
 	struct mms114_data *data;
 	struct input_dev *input_dev;
-	const void *match_data;
 	int error;
 	int i;
 
@@ -501,12 +531,10 @@ static int mms114_probe(struct i2c_client *client)
 	data->client = client;
 	data->input_dev = input_dev;
 
-	match_data = device_get_match_data(&client->dev);
-	if (!match_data)
+	data->chip = i2c_get_match_data(client);
+	if (!data->chip)
 		return -EINVAL;
 
-	data->type = (enum mms_type)match_data;
-
 	data->num_keycodes = device_property_count_u32(&client->dev,
 						       "linux,keycodes");
 	if (data->num_keycodes == -EINVAL) {
@@ -563,8 +591,7 @@ static int mms114_probe(struct i2c_client *client)
 				     0, data->props.max_y, 0, 0);
 	}
 
-	if (data->type == TYPE_MMS114 || data->type == TYPE_MMS134S ||
-	    data->type == TYPE_MMS136) {
+	if (data->chip->has_config_regs) {
 		/*
 		 * The firmware handles movement and pressure fuzz, so
 		 * don't duplicate that in software.
@@ -579,8 +606,8 @@ static int mms114_probe(struct i2c_client *client)
 	}
 
 	input_dev->name = devm_kasprintf(&client->dev, GFP_KERNEL,
-					 "MELFAS MMS%d Touchscreen",
-					 data->type);
+					 "MELFAS %s Touchscreen",
+					 data->chip->name);
 	if (!input_dev->name)
 		return -ENOMEM;
 
@@ -676,29 +703,22 @@ static int mms114_resume(struct device *dev)
 static DEFINE_SIMPLE_DEV_PM_OPS(mms114_pm_ops, mms114_suspend, mms114_resume);
 
 static const struct i2c_device_id mms114_id[] = {
-	{ .name = "mms114" },
+	{ .name = "mms114", .driver_data = (kernel_ulong_t)&mms114_descriptor },
+	{ .name = "mms134s", .driver_data = (kernel_ulong_t)&mms134s_descriptor },
+	{ .name = "mms136", .driver_data = (kernel_ulong_t)&mms136_descriptor },
+	{ .name = "mms152", .driver_data = (kernel_ulong_t)&mms152_descriptor },
+	{ .name = "mms345l", .driver_data = (kernel_ulong_t)&mms345l_descriptor },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, mms114_id);
 
 #ifdef CONFIG_OF
 static const struct of_device_id mms114_dt_match[] = {
-	{
-		.compatible = "melfas,mms114",
-		.data = (void *)TYPE_MMS114,
-	}, {
-		.compatible = "melfas,mms134s",
-		.data = (void *)TYPE_MMS134S,
-	}, {
-		.compatible = "melfas,mms136",
-		.data = (void *)TYPE_MMS136,
-	}, {
-		.compatible = "melfas,mms152",
-		.data = (void *)TYPE_MMS152,
-	}, {
-		.compatible = "melfas,mms345l",
-		.data = (void *)TYPE_MMS345L,
-	},
+	{ .compatible = "melfas,mms114", .data = &mms114_descriptor },
+	{ .compatible = "melfas,mms134s", .data = &mms134s_descriptor },
+	{ .compatible = "melfas,mms136", .data = &mms136_descriptor },
+	{ .compatible = "melfas,mms152", .data = &mms152_descriptor },
+	{ .compatible = "melfas,mms345l", .data = &mms345l_descriptor },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, mms114_dt_match);
-- 
2.54.0.1136.gdb2ca164c4-goog

Re: [PATCH 4/6] Input: mms114 - replace udelay with usleep_range

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] Race condition in `mms114_suspend` releases touches before disabling the hardware IRQ, potentially leaving touches stuck active.
- [High] Failure in `mms114_resume()` leads to unbalanced `disable_irq()` and `regulator_disable()`, causing kernel warnings and state corruption.
- [High] Copy-paste error in `mms114_setup_regs` writes the X resolution to the Y resolution register, breaking touchscreen coordinates mapping.
- [High] Hardware data bytes are read directly into a struct containing C bitfields, leading to endian-dependent memory layout corruption.
--

commit 4b0151507f5071f8f96d0af3ccffe9ae30a17c92
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: mms114 - replace udelay with usleep_range

The driver currently uses udelay(MMS114_I2C_DELAY) (50us) to ensure a
mandatory delay between I2C transfers in __mms114_read_reg() and
mms114_write_reg().

Both functions invoke underlying I2C core operations (i2c_transfer,
i2c_master_send) which acquire mutexes and sleep. Furthermore, the
interrupt handler mms114_interrupt() is registered as a threaded IRQ
handler. Since the entire execution path is fully sleepable,
busy-waiting with udelay() for 50us unnecessarily wastes CPU cycles.

Replace udelay() with usleep_range() to allow the CPU to enter low-power
states or execute other tasks during the delay.

> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index c2e006ac11960..c59aec8f2febe 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -116,7 +116,7 @@ static int __mms114_read_reg(struct mms114_data *data, u8 reg,
>  			"%s: i2c transfer failed (%d)\n", __func__, error);
>  		return error < 0 ? error : -EIO;
>  	}
> -	udelay(MMS114_I2C_DELAY);
> +	usleep_range(MMS114_I2C_DELAY, MMS114_I2C_DELAY + 50);
>  
>  	return 0;
>  }

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential race condition in the
suspend path.

drivers/input/touchscreen/mms114.c:mms114_suspend() {
...
	/* Release all touch */
	for (id = 0; id < MMS114_MAX_TOUCH; id++) {
		input_mt_slot(input_dev, id);
		input_mt_report_slot_inactive(input_dev);
	}

	input_mt_report_pointer_emulation(input_dev, true);
	input_sync(input_dev);

	guard(mutex)(&input_dev->mutex);

	if (input_device_enabled(input_dev))
		mms114_stop(data);
...
}

Because mms114_stop() disables the hardware IRQ after input_sync() has reported
the release events, is it possible for a hardware interrupt to fire between
the synthetic release and the actual IRQ disable?

Since the interrupt handler runs as a concurrent kernel thread and does not
share a lock with this early part of the suspend path, the touch could be
reported as active again right before the system sleeps. Would this leave the
touch stuck active indefinitely upon resume?

[Severity: High]
This isn't a bug introduced by this patch, but the resume error handling
might lead to unbalanced resource disabling.

drivers/input/touchscreen/mms114.c:mms114_resume() {
...
	if (input_device_enabled(input_dev)) {
		error = mms114_start(data);
		if (error)
			return error;
	}
...
}

If mms114_start() fails (e.g. due to I2C communication failure), it disables
the regulators and returns an error without enabling the IRQ. However, the
input device remains logically open.

If userspace subsequently closes the device, or another suspend cycle occurs,
mms114_stop() will be called unconditionally:

drivers/input/touchscreen/mms114.c:mms114_stop() {
...
	disable_irq(client->irq);

	error = regulator_disable(data->io_reg);
...
}

Will this unconditionally call disable_irq() and regulator_disable() on
resources that are already disabled, triggering warnings and corrupting
regulator reference counts?

[Severity: High]
This is a pre-existing issue, but I spotted a potential copy-paste error
during resolution configuration.

drivers/input/touchscreen/mms114.c:mms114_setup_regs() {
...
	val = props->max_x & 0xff;
	error = mms114_write_reg(data, MMS114_X_RESOLUTION, val);
	if (error < 0)
		return error;

	val = props->max_x & 0xff;
	error = mms114_write_reg(data, MMS114_Y_RESOLUTION, val);
...
}

Should the second register write use props->max_y instead of props->max_x?

Writing the X resolution to MMS114_Y_RESOLUTION might truncate the hardware's
internal Y resolution, breaking touch tracking and coordinate mapping on the
Y axis for screens where the dimensions differ.

[Severity: High]
This isn't a bug introduced by this patch, but there might be an endianness
problem with the touch data structure.

drivers/input/touchscreen/mms114.c:
...
struct mms114_touch {
	u8 id:4, reserved_bit4:1, type:2, pressed:1;
	u8 x_hi:4, y_hi:4;
	u8 x_lo;
	u8 y_lo;
	u8 width;
	u8 strength;
	u8 reserved[2];
} __packed;
...

This struct is used to map raw byte stream data from the I2C hardware. Since
the memory layout of C bitfields is compiler and endian-dependent, will these
bitfields be reversed on big-endian architectures?

This could result in incorrect parsing of touch coordinates, IDs, and event
types.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616050912.1531241-1-dmitry.torokhov@gmail.com?part=4

Re: [PATCH 6/6] Input: mms114 - refactor chip variant handling using descriptors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The input device name exposed to userspace has changed for the MMS134S and MMS345L variants, which risks breaking userspace configurations.
--

commit 6433e094b88447042b6bbde3852dd64f8e938510
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: mms114 - refactor chip variant handling using descriptors

This commit replaces enum-based chip variant checks with a descriptor
structure. It encapsulates variant-specific properties and callbacks into
per-chip descriptors mapped directly via device ID tables.

> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index bf01eee0560a1..006dded17eb80 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c

[ ... ]

> @@ -579,8 +606,8 @@ static int mms114_probe(struct i2c_client *client)
>  	}
>  
> -	input_dev->name = devm_kasprintf(&client->dev, GFP_KERNEL,
> -					 "MELFAS MMS%d Touchscreen",
> -					 data->type);
> +	input_dev->name = devm_kasprintf(&client->dev, GFP_KERNEL,
> +					 "MELFAS %s Touchscreen",
> +					 data->chip->name);

[Severity: Medium]
Could this string format change break userspace configurations that rely on
the exact input device name?

Previously, using the integer type, TYPE_MMS134S (134) resulted in the name
"MELFAS MMS134 Touchscreen", and TYPE_MMS345L (345) resulted in
"MELFAS MMS345 Touchscreen". 

By switching to the descriptor name string, the names now include the trailing
characters ("MELFAS MMS134S Touchscreen" and "MELFAS MMS345L Touchscreen").

Might this disrupt existing hwdb rules or udev scripts matching on the old
UAPI-visible names?

>  	if (!input_dev->name)
>  		return -ENOMEM;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616050912.1531241-1-dmitry.torokhov@gmail.com?part=6

Re: [PATCH 3/6] Input: mms114 - use appropriate register argument types

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Stack-allocated memory is passed to I2C transfer functions, which is unsafe as I2C controllers may use DMA.
- [High] The driver doesn't handle a touch ID of 0 for touchscreen events, which causes a slot underflow and corrupts the previously active slot's state.
- [High] Incorrect initialization of Y-axis resolution register.
--

commit e6fe847438e30cfef36a69ac4ec8110ef02638d0
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: mms114 - use appropriate register argument types

This commit updates the register arguments in the mms114 touch controller
driver to use 8-bit unsigned integers, matching the hardware specifications.
It also changes the value buffer pointer to void* to prevent explicit casting
when reading from the device.

> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index db23b51f4630d..c2e006ac11960 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -87,12 +87,12 @@ struct mms114_touch {
>  	u8 reserved[2];
>  } __packed;
>  
> -static int __mms114_read_reg(struct mms114_data *data, unsigned int reg,
> -			     unsigned int len, u8 *val)
> +static int __mms114_read_reg(struct mms114_data *data, u8 reg,
> +			     unsigned int len, void *val)
>  {
>  	struct i2c_client *client = data->client;
>  	struct i2c_msg xfer[2];
> -	u8 buf = reg & 0xff;
> +	u8 buf = reg;

[Severity: High]
This is a pre-existing issue, but I noticed we are using a stack-allocated
variable here.

Could this cause issues with DMA? The buf variable is allocated on the stack
and its address is assigned to xfer[0].buf for the outgoing I2C transfer.
Does this violate the DMA API prohibition on using stack-allocated memory
for I2C controllers that use DMA?

[ ... ]

> -static int mms114_write_reg(struct mms114_data *data, unsigned int reg,
> -			    unsigned int val)
> +static int mms114_write_reg(struct mms114_data *data, u8 reg, u8 val)
>  {
>  	struct i2c_client *client = data->client;
>  	u8 buf[2];
>  	int error;
>  
> -	buf[0] = reg & 0xff;
> -	buf[1] = val & 0xff;
> +	buf[0] = reg;
> +	buf[1] = val;
>  
>  	error = i2c_master_send(client, buf, 2);

[Severity: High]
This isn't a bug introduced by this patch, but this function also uses a stack
array.

Is it safe to pass the local stack array buf[2] directly to
i2c_master_send()? Does this risk stack corruption if the I2C controller
uses DMA?

[ ... ]

> @@ -242,9 +241,8 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
>  
>  	touch_size = packet_size / event_size;
>  
> -	error = __mms114_read_reg(data, MMS114_INFORMATION, packet_size,
> -			(u8 *)touch);
> -	if (error < 0)
> +	error = __mms114_read_reg(data, MMS114_INFORMATION, packet_size, touch);
> +	if (error)
>  		goto out;

[Severity: High]
This isn't a bug introduced by this patch, but the touch array is allocated
on the stack in this interrupt handler.

Since the touch array is passed as the read buffer for incoming I2C
touch events, could this lead to stack corruption if the I2C controller
performs DMA?


While reviewing this file, I also noticed a couple of other pre-existing
issues outside of this diff:

[Severity: High]
This is a pre-existing issue, but I noticed there isn't a check for touch ID
0 in mms114_process_mt():

    id = touch->id - 1;
    ...
    input_mt_slot(input_dev, id);

If the hardware sends a touch event with touch->id == 0, id would underflow
to -1. When passed to input_mt_slot(), would this cause the input core to
ignore the slot update and overwrite the state of the previously active slot?


[Severity: High]
This is a pre-existing issue, but it looks like we might be writing the
X-resolution into the Y-resolution register in mms114_setup_regs():

    val = props->max_x & 0xff;
    error = mms114_write_reg(data, MMS114_Y_RESOLUTION, val);

Does val here inadvertently use props->max_x instead of props->max_y
when writing to MMS114_Y_RESOLUTION?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616050912.1531241-1-dmitry.torokhov@gmail.com?part=3

Re: [PATCH 1/6] Input: mms114 - fix touch indexing for MMS134S and MMS136

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing validation for `touch->id == 0` causes multi-touch slot state corruption.
- [High] The devres allocation order in probe registers the input device's unregistration action before the regulators and IRQ are allocated, leading to a Use-After-Free.
- [Medium] Hardware I2C packet layout is mapped using compiler-dependent C bitfields.
--

commit d12fd2dedb0344983eaa73c88bc06f0ac0c59d09
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: mms114 - fix touch indexing for MMS134S and MMS136

This commit fixes an issue where the driver iterates through touch events
using standard array indexing, assuming an 8-byte size. It explicitly
calculates the byte offset for each event based on the device's specific
event size (6 bytes for MMS134S and MMS136).

> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index 4d748a13408d1..53ad35d61d476 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -217,7 +217,9 @@ static irqreturn_t mms114_interrupt(int irq, void *dev_id)
>  	struct mms114_data *data = dev_id;
>  	struct i2c_client *client = data->client;
>  	struct mms114_touch touch[MMS114_MAX_TOUCH];
> +	struct mms114_touch *t;

[ ... ]

>  	for (index = 0; index < touch_size; index++) {
> -		switch (touch[index].type) {
> +		t = (struct mms114_touch *)((u8 *)touch + index * event_size);
> +

[Severity: Medium]
This is a pre-existing issue, but I noticed the hardware I2C packet layout
is mapped using C bitfields:

struct mms114_touch {
        u8 id:4, reserved_bit4:1, type:2, pressed:1;
        u8 x_hi:4, y_hi:4;
...

Because the layout of bitfields within a byte is compiler and endianness
dependent, will this read garbage for the id, type, and pressed fields
if the driver is compiled for a Big Endian system?

> +		switch (t->type) {
>  		case MMS114_TYPE_TOUCHSCREEN:
> -			mms114_process_mt(data, touch + index);
> +			mms114_process_mt(data, t);

[Severity: High]
This isn't a bug introduced by this patch, but does the multi-touch slot
processing handle an empty or invalid touch ID safely?

Looking at mms114_process_mt(), the slot index is calculated without a
check for zero:

mms114_process_mt() {
        ...
        id = touch->id - 1;
        ...
        input_mt_slot(input_dev, id);

If the device reports a touch event with touch->id == 0, id will underflow
to -1. Since the input core ignores negative slot values and leaves the
active slot unchanged, would subsequent coordinate reporting functions act
on this unchanged slot and silently corrupt the multi-touch tracking?

>  			break;
>  
>  		case MMS114_TYPE_TOUCHKEY:

[ ... ]

[Severity: High]
This is a pre-existing issue, but I have a question about the device cleanup
sequence during unbind.

In mms114_probe(), devm_input_allocate_device() is called before the
regulators and IRQ are allocated:

mms114_probe() {
        ...
        input_dev = devm_input_allocate_device(&client->dev);
        ...
        data->core_reg = devm_regulator_get(&client->dev, "avdd");
}

Because devres executes cleanup actions in reverse order (LIFO), during
unbind the IRQ and regulators are freed before input_unregister_device()
is called.

If the input device is held open by userspace during unbind, does
input_unregister_device() trigger mms114_input_close() and then
mms114_stop()?

mms114_stop() {
        ...
        disable_irq(client->irq);
        error = regulator_disable(data->io_reg);
        ...
}

Could this attempt to disable a freed IRQ and access freed regulators,
causing a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616050912.1531241-1-dmitry.torokhov@gmail.com?part=1

Re: [PATCH 5/6] Input: mms114 - replace BUG() and fix alignment

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Race condition in `mms114_suspend` leading to stuck touch state.
--

commit a5a14734d553a190d315dfdcd845e9a868df4005
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Input: mms114 - replace BUG() and fix alignment

This commit avoids crashing the kernel by replacing a BUG() call with WARN_ON()
when userspace reads a write-only register. It also corrects parameter
alignment in several function calls to match kernel coding style.

> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index c59aec8f2febe..bf01eee0560a1 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential race
condition in mms114_suspend() that could leave the touchscreen stuck in a
pressed state:

drivers/input/touchscreen/mms114.c:mms114_suspend() {
    ...
	for (id = 0; id < MMS114_MAX_TOUCH; id++) {
		input_mt_slot(input_dev, id);
		input_mt_report_slot_inactive(input_dev);
	}

	input_mt_report_pointer_emulation(input_dev, true);
	input_sync(input_dev);

	guard(mutex)(&input_dev->mutex);

	if (input_device_enabled(input_dev))
		mms114_stop(data);
    ...
}

Can mms114_interrupt() execute concurrently after input_sync() but before
mms114_stop() disables the IRQ?

If a user touches the screen during this window, wouldn't the interrupt
handler report a new active touch state right after these synthetic release
events?

Because mms114_stop() then disables the IRQ, would this freeze the touch in a
pressed state throughout suspend?

When resuming, releasing the screen wouldn't generate a release interrupt,
which might leave the touchscreen stuck until reboot or until that exact
slot is touched again.

Could mms114_stop() be called before synthesizing the release events to
prevent this race?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616050912.1531241-1-dmitry.torokhov@gmail.com?part=5

Re: [PATCH 1/6] Input: mms114 - fix touch indexing for MMS134S and MMS136

[Bryam Vargas]

Hi Dmitry,

The indexing fix looks correct -- deriving the byte offset from event_size
instead of leaning on sizeof(struct mms114_touch) is the right call, and the
cast is fine since the struct is __packed (no alignment issue at the 6-byte
stride, and the consumers only touch bytes 0..5).

Two things that might be worth folding into the series:

1) Fixes: tag.  The 6-byte event path for MMS136 -- and therefore this
   stride-vs-index mismatch -- predates ab108678195f.  It was introduced in

       53fefdd1d3a3 ("Input: mms114 - support MMS136")

   which added MMS136_EVENT_SIZE and the "packet_size / MMS136_EVENT_SIZE"
   branch while the loop already indexed with the 8-byte struct stride;
   ab108678195f ("support MMS134S") only extended that branch to MMS134S.
   So MMS136 hardware has mis-parsed multi-touch since v5.13.  Tagging

       Fixes: 53fefdd1d3a3 ("Input: mms114 - support MMS136")

   (in addition to, or instead of, ab108678195f) lets stable pick it up for
   the MMS136 range as well.

2) Unbounded packet_size.  Since 1/6 already rewrites this handler: packet_size
   comes straight from the device's PACKET_SIZE register (a single byte, so
   1..255 after the "<= 0" guard) and is then used unclamped both as the read
   length

       __mms114_read_reg(data, MMS114_INFORMATION, packet_size, (u8 *)touch);

   into the 80-byte touch[MMS114_MAX_TOUCH] stack buffer, and as the divisor
   for touch_size -- which is never bounded by MMS114_MAX_TOUCH.  A device that
   reports packet_size > 80 overflows the stack buffer on the read, and even
   with the indexing fix the loop still walks past it (touch_size > 10).  A
   one-liner before the division closes both:

       if (packet_size <= 0)
               goto out;
   +   packet_size = min_t(int, packet_size, sizeof(touch));

   This one is pre-existing -- the unbounded read goes back to the original
   driver -- so it is really a separate patch in the series rather than part
   of the indexing fix, but it seemed worth raising while the code is in
   flight.

Thanks,
Bryam

Re: [PATCH 5/6] Input: mms114 - replace BUG() and fix alignment

[Bryam Vargas]

Hi Dmitry,

Replacing BUG() with WARN_ON() + return is the right call, and the rest of
the patch is fine.

One nit on the changelog though: this guard isn't reachable from userspace.
reg and len are always driver-internal constants -- there is no sysfs/debugfs/
ioctl that lets userspace pick a register, every __mms114_read_reg() call site
passes a fixed reg >= MMS114_INFORMATION (0x10), and the single MMS114_MODE_CONTROL
read is short-circuited by the cache in mms114_read_reg() before it ever reaches
__mms114_read_reg(). So the condition only fires if a *future in-kernel caller*
requests a read that spans the write-only MODE_CONTROL register -- a driver-
internal invariant, not a userspace-triggered one.

Worth tightening the wording, since that distinction is exactly what justifies
WARN_ON() here: a genuinely userspace-reachable WARN_ON() would itself be a
problem (log spam / panic_on_warn). Maybe something like:

  "Avoid taking the machine down with BUG() if a caller ever requests a read
   spanning the write-only MODE_CONTROL register; warn and return -EINVAL so
   the driver can recover."

The code is good either way.

Thanks,
Bryam

Re: [PATCH 6/6] Input: mms114 - refactor chip variant handling using descriptors

[Bryam Vargas]

Hi Dmitry,

The descriptor refactor looks good -- it preserves the per-variant behaviour
(event size, config-reg gating, get_version, fuzz handling) for all five chips.

One user-visible side effect worth a line in the changelog though: switching
input_dev->name from "MELFAS MMS%d" (the enum value) to "MELFAS %s" (chip->name)
changes the reported name for the two variants whose part number carries a
non-numeric suffix:

  MMS134S:  "MELFAS MMS134 Touchscreen"  ->  "MELFAS MMS134S Touchscreen"
  MMS345L:  "MELFAS MMS345 Touchscreen"  ->  "MELFAS MMS345L Touchscreen"

(MMS114/136/152 are byte-identical.) The old "%d" on the enum silently dropped
the S/L. That string is userspace-visible via EVIOCGNAME, /sys/class/input/*/name
and /proc/bus/input/devices, so any udev/hwdb/libinput rule keyed on the old name
would stop matching. The in-tree boards affected are ste-ux500-samsung-kyle
(melfas,mms134s) and msm8916-samsung-a5u-eur / msm8916-samsung-e5 (melfas,mms345l).

The new names are more correct, so this is just worth a heads-up in the commit
message rather than a code change -- in case an out-of-tree quirk keys on the old
truncated strings.

Thanks,
Bryam