On Thu, May 07, 2020 at 10:56:08PM -0700, Jiada Wang wrote:

?

It is a byte. Just say

	unsigned long keystates = msg[2];

+
+	for (key = 0; key < data->t15_num_keys; key++) {
+		curr_state = test_bit(key, &data->t15_keystatus);
+		new_state = test_bit(key, &keystates);
+
+		if (!curr_state && new_state) {
+			dev_dbg(dev, "T15 key press: %u\n", key);
+			__set_bit(key, &data->t15_keystatus);
+			input_event(input_dev, EV_KEY,
+				    data->t15_keymap[key], 1);
+			sync = true;
+		} else if (curr_state && !new_state) {
+			dev_dbg(dev, "T15 key release: %u\n", key);
+			__clear_bit(key, &data->t15_keystatus);
+			input_event(input_dev, EV_KEY,
+				    data->t15_keymap[key], 0);
+			sync = true;
+		}
+	}
+
+	if (sync)
+		input_sync(input_dev);

I wonder if the following is not simpler:

	unsigned long changed_keys;
	...

	changed_keys = keystates ^ data->t15_keystatus;
	for_each_set_bit(key, &changed_keys, data->t15_num_keys) {
		pressed = test_bit(key, &keystates);
		input_event(input_dev, EV_KEY,
			    data->t15_keymap[key], pressed);
		dev_dbg(dev, "T15 key %s: %u\n",
			pressed ? "press" : "release", key);
	}

	if (changed_keys)
		input_sync(input_dev);

	 data->t15_keystatus = keystates;

...

+	if (device_property_present(dev, buttons_property)) {
+		n_keys = device_property_read_u32_array(dev, buttons_property,
+							NULL, 0);
+		if (n_keys <= 0) {
+			error = n_keys < 0 ? n_keys : -EINVAL;
+			dev_err(dev, "invalid/malformed '%s' property: %d\n",
+				buttons_property, error);
+			return error;
+		}
+
+		buttonmap = devm_kmalloc_array(dev, n_keys, sizeof(*buttonmap),
+					       GFP_KERNEL);
+		if (!buttonmap)
+			return -ENOMEM;

So it is 8 keys max, I'd simply embed this into data. On 64 bit arch it
will occupy the same space as the pointer you use to reference it.

Can you also validate that we do not get too many keys in DT?

Also, set keycode/keycodemax/keycodesize in input device so userspace
can adjust keymap if needed?

Thanks.

-- 
Dmitry