2024-06-10 04:18:31

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH 1/2] Input: ims-pcu - use driver core to instantiate device attributes

Instead of manually creating driver-specific device attributes
set struct usb_driver->dev_groups pointer to have the driver core
do it.

Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/misc/ims-pcu.c | 53 +++++++++++++++++++-----------------
1 file changed, 28 insertions(+), 25 deletions(-)

diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index 408a586f8c36..0e61d969662f 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -1466,9 +1466,27 @@ static struct attribute *ims_pcu_ofn_attrs[] = {
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",
- .attrs = ims_pcu_ofn_attrs,
+ .name = "ofn",
+ .is_visible = ims_pcu_ofn_is_attr_visible,
+ .attrs = ims_pcu_ofn_attrs,
};

static void ims_pcu_irq(struct urb *urb)
@@ -1890,16 +1908,6 @@ static int ims_pcu_init_application_mode(struct ims_pcu *pcu)
/* Device appears to be operable, complete initialization */
pcu->device_no = atomic_inc_return(&device_no);

- /*
- * PCU-B devices, both GEN_1 and GEN_2 do not have OFN sensor
- */
- if (pcu->device_id != IMS_PCU_PCU_B_DEVICE_ID) {
- error = sysfs_create_group(&pcu->dev->kobj,
- &ims_pcu_ofn_attr_group);
- if (error)
- return error;
- }
-
error = ims_pcu_setup_backlight(pcu);
if (error)
return error;
@@ -1936,10 +1944,6 @@ static void ims_pcu_destroy_application_mode(struct ims_pcu *pcu)
ims_pcu_destroy_gamepad(pcu);
ims_pcu_destroy_buttons(pcu);
ims_pcu_destroy_backlight(pcu);
-
- if (pcu->device_id != IMS_PCU_PCU_B_DEVICE_ID)
- sysfs_remove_group(&pcu->dev->kobj,
- &ims_pcu_ofn_attr_group);
}
}

@@ -2031,20 +2035,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:
@@ -2070,8 +2068,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)
@@ -2130,9 +2126,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
--
2.45.2.505.gda0bf45e8d-goog



2024-06-10 04:18:36

by Dmitry Torokhov

[permalink] [raw]
Subject: [PATCH 2/2] Input: ims-pcu - switch to using cleanup functions

Start using __free() and guard() primitives to simplify the code
and error handling.

Signed-off-by: Dmitry Torokhov <[email protected]>
---
drivers/input/misc/ims-pcu.c | 135 ++++++++++++++++-------------------
1 file changed, 62 insertions(+), 73 deletions(-)

diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index 0e61d969662f..e7bd8f9858ac 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -928,9 +928,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);

@@ -954,7 +953,7 @@ static int ims_pcu_backlight_set_brightness(struct led_classdev *cdev,
__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));
@@ -963,8 +962,6 @@ static int ims_pcu_backlight_set_brightness(struct led_classdev *cdev,
"Failed to set desired brightness %u, error: %d\n",
value, error);

- mutex_unlock(&pcu->cmd_mutex);
-
return error;
}

@@ -978,7 +975,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) {
@@ -992,8 +989,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;
}

@@ -1073,24 +1068,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;
+ scoped_cond_guard(mutex, return -EINTR, &pcu->cmd_mutex) {
+ memset(field, 0, attr->field_length);
+ memcpy(field, buf, data_len);

- 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) \
@@ -1153,7 +1147,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;

@@ -1164,35 +1157,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,
@@ -1302,12 +1293,11 @@ static ssize_t ims_pcu_ofn_reg_data_show(struct device *dev,
int error;
u8 data;

- mutex_lock(&pcu->cmd_mutex);
- error = ims_pcu_read_ofn_config(pcu, pcu->ofn_reg_addr, &data);
- mutex_unlock(&pcu->cmd_mutex);
-
- if (error)
- return error;
+ 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);
}
@@ -1325,11 +1315,13 @@ static ssize_t ims_pcu_ofn_reg_data_store(struct device *dev,
if (error)
return error;

- mutex_lock(&pcu->cmd_mutex);
+ guard(mutex)(&pcu->cmd_mutex);
+
error = ims_pcu_write_ofn_config(pcu, pcu->ofn_reg_addr, value);
- mutex_unlock(&pcu->cmd_mutex);
+ if (error)
+ return error;

- return error ?: count;
+ return count;
}

static DEVICE_ATTR(reg_data, S_IRUGO | S_IWUSR,
@@ -1341,13 +1333,10 @@ static ssize_t ims_pcu_ofn_reg_addr_show(struct device *dev,
{
struct usb_interface *intf = to_usb_interface(dev);
struct ims_pcu *pcu = usb_get_intfdata(intf);
- int error;

- mutex_lock(&pcu->cmd_mutex);
- error = sysfs_emit(buf, "%x\n", pcu->ofn_reg_addr);
- mutex_unlock(&pcu->cmd_mutex);
+ guard(mutex)(&pcu->cmd_mutex);

- return error;
+ return sysfs_emit(buf, "%x\n", pcu->ofn_reg_addr);
}

static ssize_t ims_pcu_ofn_reg_addr_store(struct device *dev,
@@ -1363,9 +1352,9 @@ static ssize_t ims_pcu_ofn_reg_addr_store(struct device *dev,
if (error)
return error;

- mutex_lock(&pcu->cmd_mutex);
+ guard(mutex)(&pcu->cmd_mutex);
+
pcu->ofn_reg_addr = value;
- mutex_unlock(&pcu->cmd_mutex);

return count;
}
@@ -1390,12 +1379,11 @@ static ssize_t ims_pcu_ofn_bit_show(struct device *dev,
int error;
u8 data;

- mutex_lock(&pcu->cmd_mutex);
- error = ims_pcu_read_ofn_config(pcu, attr->addr, &data);
- mutex_unlock(&pcu->cmd_mutex);
-
- if (error)
- return error;
+ 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)));
}
@@ -1419,21 +1407,22 @@ static ssize_t ims_pcu_ofn_bit_store(struct device *dev,
if (value > 1)
return -EINVAL;

- mutex_lock(&pcu->cmd_mutex);
+ scoped_guard(mutex, &pcu->cmd_mutex) {
+ error = ims_pcu_read_ofn_config(pcu, attr->addr, &data);
+ if (error)
+ return error;

- error = ims_pcu_read_ofn_config(pcu, attr->addr, &data);
- if (!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;
}

- mutex_unlock(&pcu->cmd_mutex);
-
- return error ?: count;
+ return count;
}

#define IMS_PCU_OFN_BIT_ATTR(_field, _addr, _nr) \
--
2.45.2.505.gda0bf45e8d-goog