Commit f3efefb6 authored by Dmitry Torokhov's avatar Dmitry Torokhov

Input: yealink - simplify locking in sysfs attribute handling

The locking rules in the driver came from era when sysfs attributes
could live past the point of time when device would be unbound from
the driver, and so used module-global semaphore (potentially shared
between multiple yealink devices). Thankfully these times are long
gone and attributes will not be accessible once they are removed.

Simplify the logic by moving to per-device mutex, stop checking if
there is driver data instance attached to the interface, and use
guard notation to acquire the mutex.
Reviewed-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Link: https://lore.kernel.org/r/20240710234855.311366-2-dmitry.torokhov@gmail.comSigned-off-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
parent 295b89a6
...@@ -36,7 +36,7 @@ ...@@ -36,7 +36,7 @@
#include <linux/kernel.h> #include <linux/kernel.h>
#include <linux/slab.h> #include <linux/slab.h>
#include <linux/module.h> #include <linux/module.h>
#include <linux/rwsem.h> #include <linux/mutex.h>
#include <linux/usb/input.h> #include <linux/usb/input.h>
#include <linux/map_to_7segment.h> #include <linux/map_to_7segment.h>
...@@ -103,6 +103,8 @@ struct yealink_dev { ...@@ -103,6 +103,8 @@ struct yealink_dev {
u8 lcdMap[ARRAY_SIZE(lcdMap)]; /* state of LCD, LED ... */ u8 lcdMap[ARRAY_SIZE(lcdMap)]; /* state of LCD, LED ... */
int key_code; /* last reported key */ int key_code; /* last reported key */
struct mutex sysfs_mutex;
unsigned int shutdown:1; unsigned int shutdown:1;
int stat_ix; int stat_ix;
...@@ -548,8 +550,6 @@ static void input_close(struct input_dev *dev) ...@@ -548,8 +550,6 @@ static void input_close(struct input_dev *dev)
* sysfs interface * sysfs interface
******************************************************************************/ ******************************************************************************/
static DECLARE_RWSEM(sysfs_rwsema);
/* Interface to the 7-segments translation table aka. char set. /* Interface to the 7-segments translation table aka. char set.
*/ */
static ssize_t show_map(struct device *dev, struct device_attribute *attr, static ssize_t show_map(struct device *dev, struct device_attribute *attr,
...@@ -580,15 +580,10 @@ static ssize_t store_map(struct device *dev, struct device_attribute *attr, ...@@ -580,15 +580,10 @@ static ssize_t store_map(struct device *dev, struct device_attribute *attr,
*/ */
static ssize_t show_line(struct device *dev, char *buf, int a, int b) static ssize_t show_line(struct device *dev, char *buf, int a, int b)
{ {
struct yealink_dev *yld; struct yealink_dev *yld = dev_get_drvdata(dev);
int i; int i;
down_read(&sysfs_rwsema); guard(mutex)(&yld->sysfs_mutex);
yld = dev_get_drvdata(dev);
if (yld == NULL) {
up_read(&sysfs_rwsema);
return -ENODEV;
}
for (i = a; i < b; i++) for (i = a; i < b; i++)
*buf++ = lcdMap[i].type; *buf++ = lcdMap[i].type;
...@@ -598,7 +593,6 @@ static ssize_t show_line(struct device *dev, char *buf, int a, int b) ...@@ -598,7 +593,6 @@ static ssize_t show_line(struct device *dev, char *buf, int a, int b)
*buf++ = '\n'; *buf++ = '\n';
*buf = 0; *buf = 0;
up_read(&sysfs_rwsema);
return 3 + ((b - a) << 1); return 3 + ((b - a) << 1);
} }
...@@ -630,22 +624,16 @@ static ssize_t show_line3(struct device *dev, struct device_attribute *attr, ...@@ -630,22 +624,16 @@ static ssize_t show_line3(struct device *dev, struct device_attribute *attr,
static ssize_t store_line(struct device *dev, const char *buf, size_t count, static ssize_t store_line(struct device *dev, const char *buf, size_t count,
int el, size_t len) int el, size_t len)
{ {
struct yealink_dev *yld; struct yealink_dev *yld = dev_get_drvdata(dev);
int i; int i;
down_write(&sysfs_rwsema); guard(mutex)(&yld->sysfs_mutex);
yld = dev_get_drvdata(dev);
if (yld == NULL) {
up_write(&sysfs_rwsema);
return -ENODEV;
}
if (len > count) if (len > count)
len = count; len = count;
for (i = 0; i < len; i++) for (i = 0; i < len; i++)
setChar(yld, el++, buf[i]); setChar(yld, el++, buf[i]);
up_write(&sysfs_rwsema);
return count; return count;
} }
...@@ -675,15 +663,10 @@ static ssize_t store_line3(struct device *dev, struct device_attribute *attr, ...@@ -675,15 +663,10 @@ static ssize_t store_line3(struct device *dev, struct device_attribute *attr,
static ssize_t get_icons(struct device *dev, struct device_attribute *attr, static ssize_t get_icons(struct device *dev, struct device_attribute *attr,
char *buf) char *buf)
{ {
struct yealink_dev *yld; struct yealink_dev *yld = dev_get_drvdata(dev);
int i, ret = 1; int i, ret = 1;
down_read(&sysfs_rwsema); guard(mutex)(&yld->sysfs_mutex);
yld = dev_get_drvdata(dev);
if (yld == NULL) {
up_read(&sysfs_rwsema);
return -ENODEV;
}
for (i = 0; i < ARRAY_SIZE(lcdMap); i++) { for (i = 0; i < ARRAY_SIZE(lcdMap); i++) {
if (lcdMap[i].type != '.') if (lcdMap[i].type != '.')
...@@ -692,7 +675,7 @@ static ssize_t get_icons(struct device *dev, struct device_attribute *attr, ...@@ -692,7 +675,7 @@ static ssize_t get_icons(struct device *dev, struct device_attribute *attr,
yld->lcdMap[i] == ' ' ? " " : "on", yld->lcdMap[i] == ' ' ? " " : "on",
lcdMap[i].u.p.name); lcdMap[i].u.p.name);
} }
up_read(&sysfs_rwsema);
return ret; return ret;
} }
...@@ -700,15 +683,10 @@ static ssize_t get_icons(struct device *dev, struct device_attribute *attr, ...@@ -700,15 +683,10 @@ static ssize_t get_icons(struct device *dev, struct device_attribute *attr,
static ssize_t set_icon(struct device *dev, const char *buf, size_t count, static ssize_t set_icon(struct device *dev, const char *buf, size_t count,
int chr) int chr)
{ {
struct yealink_dev *yld; struct yealink_dev *yld = dev_get_drvdata(dev);
int i; int i;
down_write(&sysfs_rwsema); guard(mutex)(&yld->sysfs_mutex);
yld = dev_get_drvdata(dev);
if (yld == NULL) {
up_write(&sysfs_rwsema);
return -ENODEV;
}
for (i = 0; i < ARRAY_SIZE(lcdMap); i++) { for (i = 0; i < ARRAY_SIZE(lcdMap); i++) {
if (lcdMap[i].type != '.') if (lcdMap[i].type != '.')
...@@ -719,7 +697,6 @@ static ssize_t set_icon(struct device *dev, const char *buf, size_t count, ...@@ -719,7 +697,6 @@ static ssize_t set_icon(struct device *dev, const char *buf, size_t count,
} }
} }
up_write(&sysfs_rwsema);
return count; return count;
} }
...@@ -739,22 +716,16 @@ static ssize_t hide_icon(struct device *dev, struct device_attribute *attr, ...@@ -739,22 +716,16 @@ static ssize_t hide_icon(struct device *dev, struct device_attribute *attr,
*/ */
/* Stores raw ringtone data in the phone */ /* Stores raw ringtone data in the phone */
static ssize_t store_ringtone(struct device *dev, static ssize_t store_ringtone(struct device *dev, struct device_attribute *attr,
struct device_attribute *attr,
const char *buf, size_t count) const char *buf, size_t count)
{ {
struct yealink_dev *yld; struct yealink_dev *yld = dev_get_drvdata(dev);
down_write(&sysfs_rwsema); guard(mutex)(&yld->sysfs_mutex);
yld = dev_get_drvdata(dev);
if (yld == NULL) {
up_write(&sysfs_rwsema);
return -ENODEV;
}
/* TODO locking with async usb control interface??? */ /* TODO locking with async usb control interface??? */
yealink_set_ringtone(yld, (char *)buf, count); yealink_set_ringtone(yld, (char *)buf, count);
up_write(&sysfs_rwsema);
return count; return count;
} }
...@@ -835,14 +806,10 @@ static int usb_cleanup(struct yealink_dev *yld, int err) ...@@ -835,14 +806,10 @@ static int usb_cleanup(struct yealink_dev *yld, int err)
static void usb_disconnect(struct usb_interface *intf) static void usb_disconnect(struct usb_interface *intf)
{ {
struct yealink_dev *yld; struct yealink_dev *yld = usb_get_intfdata(intf);
down_write(&sysfs_rwsema);
yld = usb_get_intfdata(intf);
usb_set_intfdata(intf, NULL);
up_write(&sysfs_rwsema);
usb_cleanup(yld, 0); usb_cleanup(yld, 0);
usb_set_intfdata(intf, NULL);
} }
static int usb_probe(struct usb_interface *intf, const struct usb_device_id *id) static int usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
...@@ -870,6 +837,7 @@ static int usb_probe(struct usb_interface *intf, const struct usb_device_id *id) ...@@ -870,6 +837,7 @@ static int usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
yld->udev = udev; yld->udev = udev;
yld->intf = intf; yld->intf = intf;
mutex_init(&yld->sysfs_mutex);
yld->idev = input_dev = input_allocate_device(); yld->idev = input_dev = input_allocate_device();
if (!input_dev) if (!input_dev)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment