1 files changed, 352 insertions, 107 deletions

diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index e204f26b0011..4581f1c53644 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -1,11 +1,8 @@
+// SPDX-License-Identifier: GPL-2.0-only
 /*
  * Driver for IMS Passenger Control Unit Devices
  *
  * Copyright (C) 2013 The IMS Company
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2
- * as published by the Free Software Foundation.
  */
 
 #include <linux/completion.h>
@@ -20,7 +17,7 @@
 #include <linux/types.h>
 #include <linux/usb/input.h>
 #include <linux/usb/cdc.h>
-#include <asm/unaligned.h>
+#include <linux/unaligned.h>
 
 #define IMS_PCU_KEYMAP_LEN		32
 
@@ -39,18 +36,18 @@ struct ims_pcu_gamepad {
 
 struct ims_pcu_backlight {
 	struct led_classdev cdev;
-	struct work_struct work;
-	enum led_brightness desired_brightness;
 	char name[32];
 };
 
 #define IMS_PCU_PART_NUMBER_LEN		15
 #define IMS_PCU_SERIAL_NUMBER_LEN	8
 #define IMS_PCU_DOM_LEN			8
-#define IMS_PCU_FW_VERSION_LEN		(9 + 1)
-#define IMS_PCU_BL_VERSION_LEN		(9 + 1)
+#define IMS_PCU_FW_VERSION_LEN		16
+#define IMS_PCU_BL_VERSION_LEN		16
 #define IMS_PCU_BL_RESET_REASON_LEN	(2 + 1)
 
+#define IMS_PCU_PCU_B_DEVICE_ID		5
+
 #define IMS_PCU_BUF_SIZE		128
 
 struct ims_pcu {
@@ -68,6 +65,9 @@ struct ims_pcu {
 	char bl_version[IMS_PCU_BL_VERSION_LEN];
 	char reset_reason[IMS_PCU_BL_RESET_REASON_LEN];
 	int update_firmware_status;
+	u8 device_id;
+
+	u8 ofn_reg_addr;
 
 	struct usb_interface *ctrl_intf;
 
@@ -207,8 +207,7 @@ static int ims_pcu_setup_buttons(struct ims_pcu *pcu,
 
 	input = input_allocate_device();
 	if (!input) {
-		dev_err(pcu->dev,
-			"Not enough memory for input input device\n");
+		dev_err(pcu->dev, "Not enough memory for input device\n");
 		return -ENOMEM;
 	}
 
@@ -287,7 +286,7 @@ static int ims_pcu_setup_gamepad(struct ims_pcu *pcu)
 	struct input_dev *input;
 	int error;
 
-	gamepad = kzalloc(sizeof(struct ims_pcu_gamepad), GFP_KERNEL);
+	gamepad = kzalloc(sizeof(*gamepad), GFP_KERNEL);
 	input = input_allocate_device();
 	if (!gamepad || !input) {
 		dev_err(pcu->dev,
@@ -335,7 +334,7 @@ static int ims_pcu_setup_gamepad(struct ims_pcu *pcu)
 err_free_mem:
 	input_free_device(input);
 	kfree(gamepad);
-	return -ENOMEM;
+	return error;
 }
 
 static void ims_pcu_destroy_gamepad(struct ims_pcu *pcu)
@@ -371,6 +370,8 @@ static void ims_pcu_destroy_gamepad(struct ims_pcu *pcu)
 #define IMS_PCU_CMD_GET_DEVICE_ID	0xae
 #define IMS_PCU_CMD_SPECIAL_INFO	0xb0
 #define IMS_PCU_CMD_BOOTLOADER		0xb1	/* Pass data to bootloader */
+#define IMS_PCU_CMD_OFN_SET_CONFIG	0xb3
+#define IMS_PCU_CMD_OFN_GET_CONFIG	0xb4
 
 /* PCU responses */
 #define IMS_PCU_RSP_STATUS		0xc0
@@ -389,6 +390,9 @@ static void ims_pcu_destroy_gamepad(struct ims_pcu *pcu)
 #define IMS_PCU_RSP_GET_DEVICE_ID	0xce
 #define IMS_PCU_RSP_SPECIAL_INFO	0xd0
 #define IMS_PCU_RSP_BOOTLOADER		0xd1	/* Bootloader response */
+#define IMS_PCU_RSP_OFN_SET_CONFIG	0xd2
+#define IMS_PCU_RSP_OFN_GET_CONFIG	0xd3
+
 
 #define IMS_PCU_RSP_EVNT_BUTTONS	0xe0	/* Unsolicited, button state */
 #define IMS_PCU_GAMEPAD_MASK		0x0001ff80UL	/* Bits 7 through 16 */
@@ -642,8 +646,8 @@ static int __ims_pcu_execute_command(struct ims_pcu *pcu,
 #define IMS_PCU_BL_DATA_OFFSET		3
 
 static int __ims_pcu_execute_bl_command(struct ims_pcu *pcu,
-				        u8 command, const void *data, size_t len,
-				        u8 expected_response, int response_time)
+					u8 command, const void *data, size_t len,
+					u8 expected_response, int response_time)
 {
 	int error;
 
@@ -735,11 +739,11 @@ static int ims_pcu_switch_to_bootloader(struct ims_pcu *pcu)
 {
 	int error;
 
-	/* Execute jump to the bootoloader */
+	/* Execute jump to the bootloader */
 	error = ims_pcu_execute_command(pcu, JUMP_TO_BTLDR, NULL, 0);
 	if (error) {
 		dev_err(pcu->dev,
-			"Failure when sending JUMP TO BOOLTLOADER command, error: %d\n",
+			"Failure when sending JUMP TO BOOTLOADER command, error: %d\n",
 			error);
 		return error;
 	}
@@ -756,7 +760,7 @@ static int ims_pcu_switch_to_bootloader(struct ims_pcu *pcu)
 struct ims_pcu_flash_fmt {
 	__le32 addr;
 	u8 len;
-	u8 data[];
+	u8 data[] __counted_by(len);
 };
 
 static unsigned int ims_pcu_count_fw_records(const struct firmware *fw)
@@ -840,6 +844,12 @@ static int ims_pcu_flash_firmware(struct ims_pcu *pcu,
 		addr = be32_to_cpu(rec->addr) / 2;
 		len = be16_to_cpu(rec->len);
 
+		if (len > sizeof(pcu->cmd_buf) - 1 - sizeof(*fragment)) {
+			dev_err(pcu->dev,
+				"Invalid record length in firmware: %d\n", len);
+			return -EINVAL;
+		}
+
 		fragment = (void *)&pcu->cmd_buf[1];
 		put_unaligned_le32(addr, &fragment->addr);
 		fragment->len = len;
@@ -923,9 +933,8 @@ static void ims_pcu_process_async_firmware(const struct firmware *fw,
 		goto out;
 	}
 
-	mutex_lock(&pcu->cmd_mutex);
-	ims_pcu_handle_firmware_update(pcu, fw);
-	mutex_unlock(&pcu->cmd_mutex);
+	scoped_guard(mutex, &pcu->cmd_mutex)
+		ims_pcu_handle_firmware_update(pcu, fw);
 
 	release_firmware(fw);
 
@@ -939,36 +948,26 @@ out:
 
 #define IMS_PCU_MAX_BRIGHTNESS		31998
 
-static void ims_pcu_backlight_work(struct work_struct *work)
+static int ims_pcu_backlight_set_brightness(struct led_classdev *cdev,
+					    enum led_brightness value)
 {
 	struct ims_pcu_backlight *backlight =
-			container_of(work, struct ims_pcu_backlight, work);
+			container_of(cdev, struct ims_pcu_backlight, cdev);
 	struct ims_pcu *pcu =
 			container_of(backlight, struct ims_pcu, backlight);
-	int desired_brightness = backlight->desired_brightness;
-	__le16 br_val = cpu_to_le16(desired_brightness);
+	__le16 br_val = cpu_to_le16(value);
 	int error;
 
-	mutex_lock(&pcu->cmd_mutex);
+	guard(mutex)(&pcu->cmd_mutex);
 
 	error = ims_pcu_execute_command(pcu, SET_BRIGHTNESS,
 					&br_val, sizeof(br_val));
 	if (error && error != -ENODEV)
 		dev_warn(pcu->dev,
 			 "Failed to set desired brightness %u, error: %d\n",
-			 desired_brightness, error);
-
-	mutex_unlock(&pcu->cmd_mutex);
-}
+			 value, error);
 
-static void ims_pcu_backlight_set_brightness(struct led_classdev *cdev,
-					     enum led_brightness value)
-{
-	struct ims_pcu_backlight *backlight =
-			container_of(cdev, struct ims_pcu_backlight, cdev);
-
-	backlight->desired_brightness = value;
-	schedule_work(&backlight->work);
+	return error;
 }
 
 static enum led_brightness
@@ -981,7 +980,7 @@ ims_pcu_backlight_get_brightness(struct led_classdev *cdev)
 	int brightness;
 	int error;
 
-	mutex_lock(&pcu->cmd_mutex);
+	guard(mutex)(&pcu->cmd_mutex);
 
 	error = ims_pcu_execute_query(pcu, GET_BRIGHTNESS);
 	if (error) {
@@ -995,8 +994,6 @@ ims_pcu_backlight_get_brightness(struct led_classdev *cdev)
 			get_unaligned_le16(&pcu->cmd_buf[IMS_PCU_DATA_OFFSET]);
 	}
 
-	mutex_unlock(&pcu->cmd_mutex);
-
 	return brightness;
 }
 
@@ -1005,14 +1002,14 @@ static int ims_pcu_setup_backlight(struct ims_pcu *pcu)
 	struct ims_pcu_backlight *backlight = &pcu->backlight;
 	int error;
 
-	INIT_WORK(&backlight->work, ims_pcu_backlight_work);
 	snprintf(backlight->name, sizeof(backlight->name),
 		 "pcu%d::kbd_backlight", pcu->device_no);
 
 	backlight->cdev.name = backlight->name;
 	backlight->cdev.max_brightness = IMS_PCU_MAX_BRIGHTNESS;
 	backlight->cdev.brightness_get = ims_pcu_backlight_get_brightness;
-	backlight->cdev.brightness_set = ims_pcu_backlight_set_brightness;
+	backlight->cdev.brightness_set_blocking =
+					 ims_pcu_backlight_set_brightness;
 
 	error = led_classdev_register(pcu->dev, &backlight->cdev);
 	if (error) {
@@ -1030,7 +1027,6 @@ static void ims_pcu_destroy_backlight(struct ims_pcu *pcu)
 	struct ims_pcu_backlight *backlight = &pcu->backlight;
 
 	led_classdev_unregister(&backlight->cdev);
-	cancel_work_sync(&backlight->work);
 }
 
 
@@ -1054,7 +1050,7 @@ static ssize_t ims_pcu_attribute_show(struct device *dev,
 			container_of(dattr, struct ims_pcu_attribute, dattr);
 	char *field = (char *)pcu + attr->field_offset;
 
-	return scnprintf(buf, PAGE_SIZE, "%.*s\n", attr->field_length, field);
+	return sysfs_emit(buf, "%.*s\n", attr->field_length, field);
 }
 
 static ssize_t ims_pcu_attribute_store(struct device *dev,
@@ -1077,24 +1073,23 @@ static ssize_t ims_pcu_attribute_store(struct device *dev,
 	if (data_len > attr->field_length)
 		return -EINVAL;
 
-	error = mutex_lock_interruptible(&pcu->cmd_mutex);
-	if (error)
-		return error;
-
-	memset(field, 0, attr->field_length);
-	memcpy(field, buf, data_len);
+	scoped_cond_guard(mutex_intr, return -EINTR, &pcu->cmd_mutex) {
+		memset(field, 0, attr->field_length);
+		memcpy(field, buf, data_len);
 
-	error = ims_pcu_set_info(pcu);
+		error = ims_pcu_set_info(pcu);
 
-	/*
-	 * Even if update failed, let's fetch the info again as we just
-	 * clobbered one of the fields.
-	 */
-	ims_pcu_get_info(pcu);
+		/*
+		 * Even if update failed, let's fetch the info again as we just
+		 * clobbered one of the fields.
+		 */
+		ims_pcu_get_info(pcu);
 
-	mutex_unlock(&pcu->cmd_mutex);
+		if (error)
+			return error;
+	}
 
-	return error < 0 ? error : count;
+	return count;
 }
 
 #define IMS_PCU_ATTR(_field, _mode)					\
@@ -1157,7 +1152,6 @@ static ssize_t ims_pcu_update_firmware_store(struct device *dev,
 {
 	struct usb_interface *intf = to_usb_interface(dev);
 	struct ims_pcu *pcu = usb_get_intfdata(intf);
-	const struct firmware *fw = NULL;
 	int value;
 	int error;
 
@@ -1168,35 +1162,33 @@ static ssize_t ims_pcu_update_firmware_store(struct device *dev,
 	if (value != 1)
 		return -EINVAL;
 
-	error = mutex_lock_interruptible(&pcu->cmd_mutex);
-	if (error)
-		return error;
-
+	const struct firmware *fw __free(firmware) = NULL;
 	error = request_ihex_firmware(&fw, IMS_PCU_FIRMWARE_NAME, pcu->dev);
 	if (error) {
 		dev_err(pcu->dev, "Failed to request firmware %s, error: %d\n",
 			IMS_PCU_FIRMWARE_NAME, error);
-		goto out;
+		return error;
 	}
 
-	/*
-	 * If we are already in bootloader mode we can proceed with
-	 * flashing the firmware.
-	 *
-	 * If we are in application mode, then we need to switch into
-	 * bootloader mode, which will cause the device to disconnect
-	 * and reconnect as different device.
-	 */
-	if (pcu->bootloader_mode)
-		error = ims_pcu_handle_firmware_update(pcu, fw);
-	else
-		error = ims_pcu_switch_to_bootloader(pcu);
+	scoped_cond_guard(mutex_intr, return -EINTR, &pcu->cmd_mutex) {
+		/*
+		 * If we are already in bootloader mode we can proceed with
+		 * flashing the firmware.
+		 *
+		 * If we are in application mode, then we need to switch into
+		 * bootloader mode, which will cause the device to disconnect
+		 * and reconnect as different device.
+		 */
+		if (pcu->bootloader_mode)
+			error = ims_pcu_handle_firmware_update(pcu, fw);
+		else
+			error = ims_pcu_switch_to_bootloader(pcu);
 
-	release_firmware(fw);
+		if (error)
+			return error;
+	}
 
-out:
-	mutex_unlock(&pcu->cmd_mutex);
-	return error ?: count;
+	return count;
 }
 
 static DEVICE_ATTR(update_firmware, S_IWUSR,
@@ -1210,7 +1202,7 @@ ims_pcu_update_firmware_status_show(struct device *dev,
 	struct usb_interface *intf = to_usb_interface(dev);
 	struct ims_pcu *pcu = usb_get_intfdata(intf);
 
-	return scnprintf(buf, PAGE_SIZE, "%d\n", pcu->update_firmware_status);
+	return sysfs_emit(buf, "%d\n", pcu->update_firmware_status);
 }
 
 static DEVICE_ATTR(update_firmware_status, S_IRUGO,
@@ -1232,7 +1224,7 @@ static struct attribute *ims_pcu_attrs[] = {
 static umode_t ims_pcu_is_attr_visible(struct kobject *kobj,
 				       struct attribute *attr, int n)
 {
-	struct device *dev = container_of(kobj, struct device, kobj);
+	struct device *dev = kobj_to_dev(kobj);
 	struct usb_interface *intf = to_usb_interface(dev);
 	struct ims_pcu *pcu = usb_get_intfdata(intf);
 	umode_t mode = attr->mode;
@@ -1251,11 +1243,246 @@ static umode_t ims_pcu_is_attr_visible(struct kobject *kobj,
 	return mode;
 }
 
-static struct attribute_group ims_pcu_attr_group = {
+static const struct attribute_group ims_pcu_attr_group = {
 	.is_visible	= ims_pcu_is_attr_visible,
 	.attrs		= ims_pcu_attrs,
 };
 
+/* Support for a separate OFN attribute group */
+
+#define OFN_REG_RESULT_OFFSET	2
+
+static int ims_pcu_read_ofn_config(struct ims_pcu *pcu, u8 addr, u8 *data)
+{
+	int error;
+	s16 result;
+
+	error = ims_pcu_execute_command(pcu, OFN_GET_CONFIG,
+					&addr, sizeof(addr));
+	if (error)
+		return error;
+
+	result = (s16)get_unaligned_le16(pcu->cmd_buf + OFN_REG_RESULT_OFFSET);
+	if (result < 0)
+		return -EIO;
+
+	/* We only need LSB */
+	*data = pcu->cmd_buf[OFN_REG_RESULT_OFFSET];
+	return 0;
+}
+
+static int ims_pcu_write_ofn_config(struct ims_pcu *pcu, u8 addr, u8 data)
+{
+	u8 buffer[] = { addr, data };
+	int error;
+	s16 result;
+
+	error = ims_pcu_execute_command(pcu, OFN_SET_CONFIG,
+					&buffer, sizeof(buffer));
+	if (error)
+		return error;
+
+	result = (s16)get_unaligned_le16(pcu->cmd_buf + OFN_REG_RESULT_OFFSET);
+	if (result < 0)
+		return -EIO;
+
+	return 0;
+}
+
+static ssize_t ims_pcu_ofn_reg_data_show(struct device *dev,
+					 struct device_attribute *dattr,
+					 char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct ims_pcu *pcu = usb_get_intfdata(intf);
+	int error;
+	u8 data;
+
+	scoped_guard(mutex, &pcu->cmd_mutex) {
+		error = ims_pcu_read_ofn_config(pcu, pcu->ofn_reg_addr, &data);
+		if (error)
+			return error;
+	}
+
+	return sysfs_emit(buf, "%x\n", data);
+}
+
+static ssize_t ims_pcu_ofn_reg_data_store(struct device *dev,
+					  struct device_attribute *dattr,
+					  const char *buf, size_t count)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct ims_pcu *pcu = usb_get_intfdata(intf);
+	int error;
+	u8 value;
+
+	error = kstrtou8(buf, 0, &value);
+	if (error)
+		return error;
+
+	guard(mutex)(&pcu->cmd_mutex);
+
+	error = ims_pcu_write_ofn_config(pcu, pcu->ofn_reg_addr, value);
+	if (error)
+		return error;
+
+	return count;
+}
+
+static DEVICE_ATTR(reg_data, S_IRUGO | S_IWUSR,
+		   ims_pcu_ofn_reg_data_show, ims_pcu_ofn_reg_data_store);
+
+static ssize_t ims_pcu_ofn_reg_addr_show(struct device *dev,
+					 struct device_attribute *dattr,
+					 char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct ims_pcu *pcu = usb_get_intfdata(intf);
+
+	guard(mutex)(&pcu->cmd_mutex);
+
+	return sysfs_emit(buf, "%x\n", pcu->ofn_reg_addr);
+}
+
+static ssize_t ims_pcu_ofn_reg_addr_store(struct device *dev,
+					  struct device_attribute *dattr,
+					  const char *buf, size_t count)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct ims_pcu *pcu = usb_get_intfdata(intf);
+	int error;
+	u8 value;
+
+	error = kstrtou8(buf, 0, &value);
+	if (error)
+		return error;
+
+	guard(mutex)(&pcu->cmd_mutex);
+
+	pcu->ofn_reg_addr = value;
+
+	return count;
+}
+
+static DEVICE_ATTR(reg_addr, S_IRUGO | S_IWUSR,
+		   ims_pcu_ofn_reg_addr_show, ims_pcu_ofn_reg_addr_store);
+
+struct ims_pcu_ofn_bit_attribute {
+	struct device_attribute dattr;
+	u8 addr;
+	u8 nr;
+};
+
+static ssize_t ims_pcu_ofn_bit_show(struct device *dev,
+				    struct device_attribute *dattr,
+				    char *buf)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct ims_pcu *pcu = usb_get_intfdata(intf);
+	struct ims_pcu_ofn_bit_attribute *attr =
+		container_of(dattr, struct ims_pcu_ofn_bit_attribute, dattr);
+	int error;
+	u8 data;
+
+	scoped_guard(mutex, &pcu->cmd_mutex) {
+		error = ims_pcu_read_ofn_config(pcu, attr->addr, &data);
+		if (error)
+			return error;
+	}
+
+	return sysfs_emit(buf, "%d\n", !!(data & (1 << attr->nr)));
+}
+
+static ssize_t ims_pcu_ofn_bit_store(struct device *dev,
+				     struct device_attribute *dattr,
+				     const char *buf, size_t count)
+{
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct ims_pcu *pcu = usb_get_intfdata(intf);
+	struct ims_pcu_ofn_bit_attribute *attr =
+		container_of(dattr, struct ims_pcu_ofn_bit_attribute, dattr);
+	int error;
+	int value;
+	u8 data;
+
+	error = kstrtoint(buf, 0, &value);
+	if (error)
+		return error;
+
+	if (value > 1)
+		return -EINVAL;
+
+	scoped_guard(mutex, &pcu->cmd_mutex) {
+		error = ims_pcu_read_ofn_config(pcu, attr->addr, &data);
+		if (error)
+			return error;
+
+		if (value)
+			data |= 1U << attr->nr;
+		else
+			data &= ~(1U << attr->nr);
+
+		error = ims_pcu_write_ofn_config(pcu, attr->addr, data);
+		if (error)
+			return error;
+	}
+
+	return count;
+}
+
+#define IMS_PCU_OFN_BIT_ATTR(_field, _addr, _nr)			\
+struct ims_pcu_ofn_bit_attribute ims_pcu_ofn_attr_##_field = {		\
+	.dattr = __ATTR(_field, S_IWUSR | S_IRUGO,			\
+			ims_pcu_ofn_bit_show, ims_pcu_ofn_bit_store),	\
+	.addr = _addr,							\
+	.nr = _nr,							\
+}
+
+static IMS_PCU_OFN_BIT_ATTR(engine_enable,   0x60, 7);
+static IMS_PCU_OFN_BIT_ATTR(speed_enable,    0x60, 6);
+static IMS_PCU_OFN_BIT_ATTR(assert_enable,   0x60, 5);
+static IMS_PCU_OFN_BIT_ATTR(xyquant_enable,  0x60, 4);
+static IMS_PCU_OFN_BIT_ATTR(xyscale_enable,  0x60, 1);
+
+static IMS_PCU_OFN_BIT_ATTR(scale_x2,        0x63, 6);
+static IMS_PCU_OFN_BIT_ATTR(scale_y2,        0x63, 7);
+
+static struct attribute *ims_pcu_ofn_attrs[] = {
+	&dev_attr_reg_data.attr,
+	&dev_attr_reg_addr.attr,
+	&ims_pcu_ofn_attr_engine_enable.dattr.attr,
+	&ims_pcu_ofn_attr_speed_enable.dattr.attr,
+	&ims_pcu_ofn_attr_assert_enable.dattr.attr,
+	&ims_pcu_ofn_attr_xyquant_enable.dattr.attr,
+	&ims_pcu_ofn_attr_xyscale_enable.dattr.attr,
+	&ims_pcu_ofn_attr_scale_x2.dattr.attr,
+	&ims_pcu_ofn_attr_scale_y2.dattr.attr,
+	NULL
+};
+
+static umode_t ims_pcu_ofn_is_attr_visible(struct kobject *kobj,
+					   struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct usb_interface *intf = to_usb_interface(dev);
+	struct ims_pcu *pcu = usb_get_intfdata(intf);
+	umode_t mode = attr->mode;
+
+	/*
+	 * PCU-B devices, both GEN_1 and GEN_2 do not have OFN sensor.
+	 */
+	if (pcu->bootloader_mode || pcu->device_id == IMS_PCU_PCU_B_DEVICE_ID)
+		mode = 0;
+
+	return mode;
+}
+
+static const struct attribute_group ims_pcu_ofn_attr_group = {
+	.name		= "ofn",
+	.is_visible	= ims_pcu_ofn_is_attr_visible,
+	.attrs		= ims_pcu_ofn_attrs,
+};
+
 static void ims_pcu_irq(struct urb *urb)
 {
 	struct ims_pcu *pcu = urb->context;
@@ -1337,6 +1564,7 @@ static int ims_pcu_buffers_alloc(struct ims_pcu *pcu)
 	if (!pcu->urb_ctrl_buf) {
 		dev_err(pcu->dev,
 			"Failed to allocate memory for read buffer\n");
+		error = -ENOMEM;
 		goto err_free_urb_out_buf;
 	}
 
@@ -1405,13 +1633,25 @@ ims_pcu_get_cdc_union_desc(struct usb_interface *intf)
 		return NULL;
 	}
 
-	while (buflen > 0) {
+	while (buflen >= sizeof(*union_desc)) {
 		union_desc = (struct usb_cdc_union_desc *)buf;
 
+		if (union_desc->bLength > buflen) {
+			dev_err(&intf->dev, "Too large descriptor\n");
+			return NULL;
+		}
+
 		if (union_desc->bDescriptorType == USB_DT_CS_INTERFACE &&
 		    union_desc->bDescriptorSubType == USB_CDC_UNION_TYPE) {
 			dev_dbg(&intf->dev, "Found union header\n");
-			return union_desc;
+
+			if (union_desc->bLength >= sizeof(*union_desc))
+				return union_desc;
+
+			dev_err(&intf->dev,
+				"Union descriptor too short (%d vs %zd)\n",
+				union_desc->bLength, sizeof(*union_desc));
+			return NULL;
 		}
 
 		buflen -= union_desc->bLength;
@@ -1433,13 +1673,21 @@ static int ims_pcu_parse_cdc_data(struct usb_interface *intf, struct ims_pcu *pc
 
 	pcu->ctrl_intf = usb_ifnum_to_if(pcu->udev,
 					 union_desc->bMasterInterface0);
+	if (!pcu->ctrl_intf)
+		return -EINVAL;
 
 	alt = pcu->ctrl_intf->cur_altsetting;
+
+	if (alt->desc.bNumEndpoints < 1)
+		return -ENODEV;
+
 	pcu->ep_ctrl = &alt->endpoint[0].desc;
 	pcu->max_ctrl_size = usb_endpoint_maxp(pcu->ep_ctrl);
 
 	pcu->data_intf = usb_ifnum_to_if(pcu->udev,
 					 union_desc->bSlaveInterface0);
+	if (!pcu->data_intf)
+		return -EINVAL;
 
 	alt = pcu->data_intf->cur_altsetting;
 	if (alt->desc.bNumEndpoints != 2) {
@@ -1621,10 +1869,9 @@ static int ims_pcu_identify_type(struct ims_pcu *pcu, u8 *device_id)
 
 static int ims_pcu_init_application_mode(struct ims_pcu *pcu)
 {
-	static atomic_t device_no = ATOMIC_INIT(0);
+	static atomic_t device_no = ATOMIC_INIT(-1);
 
 	const struct ims_pcu_device_info *info;
-	u8 device_id;
 	int error;
 
 	error = ims_pcu_get_device_info(pcu);
@@ -1633,7 +1880,7 @@ static int ims_pcu_init_application_mode(struct ims_pcu *pcu)
 		return error;
 	}
 
-	error = ims_pcu_identify_type(pcu, &device_id);
+	error = ims_pcu_identify_type(pcu, &pcu->device_id);
 	if (error) {
 		dev_err(pcu->dev,
 			"Failed to identify device, error: %d\n", error);
@@ -1645,21 +1892,21 @@ static int ims_pcu_init_application_mode(struct ims_pcu *pcu)
 		return 0;
 	}
 
-	if (device_id >= ARRAY_SIZE(ims_pcu_device_info) ||
-	    !ims_pcu_device_info[device_id].keymap) {
-		dev_err(pcu->dev, "Device ID %d is not valid\n", device_id);
+	if (pcu->device_id >= ARRAY_SIZE(ims_pcu_device_info) ||
+	    !ims_pcu_device_info[pcu->device_id].keymap) {
+		dev_err(pcu->dev, "Device ID %d is not valid\n", pcu->device_id);
 		/* Same as above, punt to userspace */
 		return 0;
 	}
 
 	/* Device appears to be operable, complete initialization */
-	pcu->device_no = atomic_inc_return(&device_no) - 1;
+	pcu->device_no = atomic_inc_return(&device_no);
 
 	error = ims_pcu_setup_backlight(pcu);
 	if (error)
 		return error;
 
-	info = &ims_pcu_device_info[device_id];
+	info = &ims_pcu_device_info[pcu->device_id];
 	error = ims_pcu_setup_buttons(pcu, info->keymap, info->keymap_len);
 	if (error)
 		goto err_destroy_backlight;
@@ -1674,10 +1921,10 @@ static int ims_pcu_init_application_mode(struct ims_pcu *pcu)
 
 	return 0;
 
-err_destroy_backlight:
-	ims_pcu_destroy_backlight(pcu);
 err_destroy_buttons:
 	ims_pcu_destroy_buttons(pcu);
+err_destroy_backlight:
+	ims_pcu_destroy_backlight(pcu);
 	return error;
 }
 
@@ -1744,7 +1991,7 @@ static int ims_pcu_probe(struct usb_interface *intf,
 	struct ims_pcu *pcu;
 	int error;
 
-	pcu = kzalloc(sizeof(struct ims_pcu), GFP_KERNEL);
+	pcu = kzalloc(sizeof(*pcu), GFP_KERNEL);
 	if (!pcu)
 		return -ENOMEM;
 
@@ -1769,7 +2016,6 @@ static int ims_pcu_probe(struct usb_interface *intf,
 	}
 
 	usb_set_intfdata(pcu->ctrl_intf, pcu);
-	usb_set_intfdata(pcu->data_intf, pcu);
 
 	error = ims_pcu_buffers_alloc(pcu);
 	if (error)
@@ -1783,20 +2029,14 @@ static int ims_pcu_probe(struct usb_interface *intf,
 	if (error)
 		goto err_stop_io;
 
-	error = sysfs_create_group(&intf->dev.kobj, &ims_pcu_attr_group);
-	if (error)
-		goto err_stop_io;
-
 	error = pcu->bootloader_mode ?
 			ims_pcu_init_bootloader_mode(pcu) :
 			ims_pcu_init_application_mode(pcu);
 	if (error)
-		goto err_remove_sysfs;
+		goto err_stop_io;
 
 	return 0;
 
-err_remove_sysfs:
-	sysfs_remove_group(&intf->dev.kobj, &ims_pcu_attr_group);
 err_stop_io:
 	ims_pcu_stop_io(pcu);
 err_free_buffers:
@@ -1822,8 +2062,6 @@ static void ims_pcu_disconnect(struct usb_interface *intf)
 	if (alt->desc.bInterfaceClass != USB_CLASS_COMM)
 		return;
 
-	sysfs_remove_group(&intf->dev.kobj, &ims_pcu_attr_group);
-
 	ims_pcu_stop_io(pcu);
 
 	if (pcu->bootloader_mode)
@@ -1882,9 +2120,16 @@ static const struct usb_device_id ims_pcu_id_table[] = {
 	{ }
 };
 
+static const struct attribute_group *ims_pcu_sysfs_groups[] = {
+	&ims_pcu_attr_group,
+	&ims_pcu_ofn_attr_group,
+	NULL
+};
+
 static struct usb_driver ims_pcu_driver = {
 	.name			= "ims_pcu",
 	.id_table		= ims_pcu_id_table,
+	.dev_groups		= ims_pcu_sysfs_groups,
 	.probe			= ims_pcu_probe,
 	.disconnect		= ims_pcu_disconnect,
 #ifdef CONFIG_PM