From a99548b8343a46c1050fc041314bbbd27065169d Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 10:55:39 +0200 Subject: HID: cp2112: destroy mutex on driver detach Use the devres variant of mutex_init() in order to free resources allocated with mutex debugging enabled. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Signed-off-by: Jiri Kosina --- drivers/hid/hid-cp2112.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c index f4c8d981aa0a..a001b9acd2d4 100644 --- a/drivers/hid/hid-cp2112.c +++ b/drivers/hid/hid-cp2112.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -1205,7 +1206,11 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) if (!dev->in_out_buffer) return -ENOMEM; - mutex_init(&dev->lock); + ret = devm_mutex_init(&hdev->dev, &dev->lock); + if (ret) { + hid_err(hdev, "mutex init failed\n"); + return ret; + } ret = hid_parse(hdev); if (ret) { -- cgit v1.2.3-59-g8ed1b From 837b05fea0752ee900abe57253d9f79ca46dd227 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 10:55:40 +0200 Subject: HID: cp2112: hold the lock for the entire direction_output() call We currently take the lock, set direction to output, release it, reacquire it and set the desired value. That doesn't look correct. Introduce a helper function that sets the value without taking the lock and use it where applicable in order to combine the critical sections. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Signed-off-by: Jiri Kosina --- drivers/hid/hid-cp2112.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c index a001b9acd2d4..408c865efdd4 100644 --- a/drivers/hid/hid-cp2112.c +++ b/drivers/hid/hid-cp2112.c @@ -17,6 +17,7 @@ */ #include +#include #include #include #include @@ -218,15 +219,13 @@ exit: return ret; } -static void cp2112_gpio_set(struct gpio_chip *chip, unsigned offset, int value) +static void cp2112_gpio_set_unlocked(struct cp2112_device *dev, + unsigned int offset, int value) { - struct cp2112_device *dev = gpiochip_get_data(chip); struct hid_device *hdev = dev->hdev; u8 *buf = dev->in_out_buffer; int ret; - mutex_lock(&dev->lock); - buf[0] = CP2112_GPIO_SET; buf[1] = value ? CP2112_GPIO_ALL_GPIO_MASK : 0; buf[2] = BIT(offset); @@ -236,8 +235,16 @@ static void cp2112_gpio_set(struct gpio_chip *chip, unsigned offset, int value) HID_REQ_SET_REPORT); if (ret < 0) hid_err(hdev, "error setting GPIO values: %d\n", ret); +} - mutex_unlock(&dev->lock); +static void cp2112_gpio_set(struct gpio_chip *chip, unsigned int offset, + int value) +{ + struct cp2112_device *dev = gpiochip_get_data(chip); + + guard(mutex)(&dev->lock); + + return cp2112_gpio_set_unlocked(dev, offset, value); } static int cp2112_gpio_get_all(struct gpio_chip *chip) @@ -306,13 +313,13 @@ static int cp2112_gpio_direction_output(struct gpio_chip *chip, goto fail; } - mutex_unlock(&dev->lock); - /* * Set gpio value when output direction is already set, * as specified in AN495, Rev. 0.2, cpt. 4.4 */ - cp2112_gpio_set(chip, offset, value); + cp2112_gpio_set_unlocked(dev, offset, value); + + mutex_unlock(&dev->lock); return 0; -- cgit v1.2.3-59-g8ed1b From 4c49d905ca434d54e399de6f0083b8a5ef0bbdf1 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 10:55:41 +0200 Subject: HID: cp2112: use lock guards Simplify the code by using the lock guards from linux/cleanup.h throughout the driver. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Signed-off-by: Jiri Kosina --- drivers/hid/hid-cp2112.c | 38 +++++++++++--------------------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c index 408c865efdd4..92dd891b7b59 100644 --- a/drivers/hid/hid-cp2112.c +++ b/drivers/hid/hid-cp2112.c @@ -187,7 +187,7 @@ static int cp2112_gpio_direction_input(struct gpio_chip *chip, unsigned offset) u8 *buf = dev->in_out_buffer; int ret; - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, CP2112_GPIO_CONFIG_LENGTH, HID_FEATURE_REPORT, @@ -196,7 +196,7 @@ static int cp2112_gpio_direction_input(struct gpio_chip *chip, unsigned offset) hid_err(hdev, "error requesting GPIO config: %d\n", ret); if (ret >= 0) ret = -EIO; - goto exit; + return ret; } buf[1] &= ~BIT(offset); @@ -209,14 +209,10 @@ static int cp2112_gpio_direction_input(struct gpio_chip *chip, unsigned offset) hid_err(hdev, "error setting GPIO config: %d\n", ret); if (ret >= 0) ret = -EIO; - goto exit; + return ret; } - ret = 0; - -exit: - mutex_unlock(&dev->lock); - return ret; + return 0; } static void cp2112_gpio_set_unlocked(struct cp2112_device *dev, @@ -244,7 +240,7 @@ static void cp2112_gpio_set(struct gpio_chip *chip, unsigned int offset, guard(mutex)(&dev->lock); - return cp2112_gpio_set_unlocked(dev, offset, value); + cp2112_gpio_set_unlocked(dev, offset, value); } static int cp2112_gpio_get_all(struct gpio_chip *chip) @@ -254,23 +250,17 @@ static int cp2112_gpio_get_all(struct gpio_chip *chip) u8 *buf = dev->in_out_buffer; int ret; - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); ret = hid_hw_raw_request(hdev, CP2112_GPIO_GET, buf, CP2112_GPIO_GET_LENGTH, HID_FEATURE_REPORT, HID_REQ_GET_REPORT); if (ret != CP2112_GPIO_GET_LENGTH) { hid_err(hdev, "error requesting GPIO values: %d\n", ret); - ret = ret < 0 ? ret : -EIO; - goto exit; + return ret < 0 ? ret : -EIO; } - ret = buf[1]; - -exit: - mutex_unlock(&dev->lock); - - return ret; + return buf[1]; } static int cp2112_gpio_get(struct gpio_chip *chip, unsigned int offset) @@ -292,14 +282,14 @@ static int cp2112_gpio_direction_output(struct gpio_chip *chip, u8 *buf = dev->in_out_buffer; int ret; - mutex_lock(&dev->lock); + guard(mutex)(&dev->lock); ret = hid_hw_raw_request(hdev, CP2112_GPIO_CONFIG, buf, CP2112_GPIO_CONFIG_LENGTH, HID_FEATURE_REPORT, HID_REQ_GET_REPORT); if (ret != CP2112_GPIO_CONFIG_LENGTH) { hid_err(hdev, "error requesting GPIO config: %d\n", ret); - goto fail; + return ret < 0 ? ret : -EIO; } buf[1] |= 1 << offset; @@ -310,7 +300,7 @@ static int cp2112_gpio_direction_output(struct gpio_chip *chip, HID_REQ_SET_REPORT); if (ret < 0) { hid_err(hdev, "error setting GPIO config: %d\n", ret); - goto fail; + return ret; } /* @@ -319,13 +309,7 @@ static int cp2112_gpio_direction_output(struct gpio_chip *chip, */ cp2112_gpio_set_unlocked(dev, offset, value); - mutex_unlock(&dev->lock); - return 0; - -fail: - mutex_unlock(&dev->lock); - return ret < 0 ? ret : -EIO; } static int cp2112_hid_get(struct hid_device *hdev, unsigned char report_number, -- cgit v1.2.3-59-g8ed1b From 6485543488a6d35e9f24c6f50cb63710446d8aab Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 10:55:42 +0200 Subject: HID: cp2112: use new line value setter callbacks struct gpio_chip now has callbacks for setting line values that return an integer, allowing to indicate failures. Convert the driver to using them. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Signed-off-by: Jiri Kosina --- drivers/hid/hid-cp2112.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c index 92dd891b7b59..234fa82eab07 100644 --- a/drivers/hid/hid-cp2112.c +++ b/drivers/hid/hid-cp2112.c @@ -215,8 +215,8 @@ static int cp2112_gpio_direction_input(struct gpio_chip *chip, unsigned offset) return 0; } -static void cp2112_gpio_set_unlocked(struct cp2112_device *dev, - unsigned int offset, int value) +static int cp2112_gpio_set_unlocked(struct cp2112_device *dev, + unsigned int offset, int value) { struct hid_device *hdev = dev->hdev; u8 *buf = dev->in_out_buffer; @@ -231,16 +231,18 @@ static void cp2112_gpio_set_unlocked(struct cp2112_device *dev, HID_REQ_SET_REPORT); if (ret < 0) hid_err(hdev, "error setting GPIO values: %d\n", ret); + + return ret; } -static void cp2112_gpio_set(struct gpio_chip *chip, unsigned int offset, - int value) +static int cp2112_gpio_set(struct gpio_chip *chip, unsigned int offset, + int value) { struct cp2112_device *dev = gpiochip_get_data(chip); guard(mutex)(&dev->lock); - cp2112_gpio_set_unlocked(dev, offset, value); + return cp2112_gpio_set_unlocked(dev, offset, value); } static int cp2112_gpio_get_all(struct gpio_chip *chip) @@ -1286,7 +1288,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id) dev->gc.label = "cp2112_gpio"; dev->gc.direction_input = cp2112_gpio_direction_input; dev->gc.direction_output = cp2112_gpio_direction_output; - dev->gc.set = cp2112_gpio_set; + dev->gc.set_rv = cp2112_gpio_set; dev->gc.get = cp2112_gpio_get; dev->gc.base = -1; dev->gc.ngpio = CP2112_GPIO_MAX_GPIO; -- cgit v1.2.3-59-g8ed1b From 9815a423613327be32f524a5162779ce4900c5b4 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 10:55:43 +0200 Subject: HID: mcp2200: use new line value setter callbacks struct gpio_chip now has callbacks for setting line values that return an integer, allowing to indicate failures. Convert the driver to using them. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Signed-off-by: Jiri Kosina --- drivers/hid/hid-mcp2200.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/drivers/hid/hid-mcp2200.c b/drivers/hid/hid-mcp2200.c index bf57f7f6caa0..e6ea0a2140eb 100644 --- a/drivers/hid/hid-mcp2200.c +++ b/drivers/hid/hid-mcp2200.c @@ -127,8 +127,8 @@ static int mcp_cmd_read_all(struct mcp2200 *mcp) return mcp->status; } -static void mcp_set_multiple(struct gpio_chip *gc, unsigned long *mask, - unsigned long *bits) +static int mcp_set_multiple(struct gpio_chip *gc, unsigned long *mask, + unsigned long *bits) { struct mcp2200 *mcp = gpiochip_get_data(gc); u8 value; @@ -150,16 +150,20 @@ static void mcp_set_multiple(struct gpio_chip *gc, unsigned long *mask, if (status == sizeof(struct mcp_set_clear_outputs)) mcp->gpio_val = value; + else + status = -EIO; mutex_unlock(&mcp->lock); + + return status; } -static void mcp_set(struct gpio_chip *gc, unsigned int gpio_nr, int value) +static int mcp_set(struct gpio_chip *gc, unsigned int gpio_nr, int value) { unsigned long mask = 1 << gpio_nr; unsigned long bmap_value = value << gpio_nr; - mcp_set_multiple(gc, &mask, &bmap_value); + return mcp_set_multiple(gc, &mask, &bmap_value); } static int mcp_get_multiple(struct gpio_chip *gc, unsigned long *mask, @@ -263,9 +267,10 @@ static int mcp_direction_output(struct gpio_chip *gc, unsigned int gpio_nr, bmap_value = value << gpio_nr; ret = mcp_set_direction(gc, gpio_nr, MCP2200_DIR_OUT); - if (!ret) - mcp_set_multiple(gc, &mask, &bmap_value); - return ret; + if (ret) + return ret; + + return mcp_set_multiple(gc, &mask, &bmap_value); } static const struct gpio_chip template_chip = { @@ -274,8 +279,8 @@ static const struct gpio_chip template_chip = { .get_direction = mcp_get_direction, .direction_input = mcp_direction_input, .direction_output = mcp_direction_output, - .set = mcp_set, - .set_multiple = mcp_set_multiple, + .set_rv = mcp_set, + .set_multiple_rv = mcp_set_multiple, .get = mcp_get, .get_multiple = mcp_get_multiple, .base = -1, -- cgit v1.2.3-59-g8ed1b From 31a78afda1ef7f2f2ced5acd99d8bc5edd0325f8 Mon Sep 17 00:00:00 2001 From: Bartosz Golaszewski Date: Wed, 23 Apr 2025 10:55:44 +0200 Subject: HID: mcp2221: use new line value setter callbacks struct gpio_chip now has callbacks for setting line values that return an integer, allowing to indicate failures. Convert the driver to using them. Signed-off-by: Bartosz Golaszewski Reviewed-by: Linus Walleij Signed-off-by: Jiri Kosina --- drivers/hid/hid-mcp2221.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c index 0f93c22a479f..6c0ac14f11a6 100644 --- a/drivers/hid/hid-mcp2221.c +++ b/drivers/hid/hid-mcp2221.c @@ -624,10 +624,10 @@ static int mcp_gpio_get(struct gpio_chip *gc, return ret; } -static void mcp_gpio_set(struct gpio_chip *gc, - unsigned int offset, int value) +static int mcp_gpio_set(struct gpio_chip *gc, unsigned int offset, int value) { struct mcp2221 *mcp = gpiochip_get_data(gc); + int ret; memset(mcp->txbuf, 0, 18); mcp->txbuf[0] = MCP2221_GPIO_SET; @@ -638,8 +638,10 @@ static void mcp_gpio_set(struct gpio_chip *gc, mcp->txbuf[mcp->gp_idx] = !!value; mutex_lock(&mcp->lock); - mcp_send_data_req_status(mcp, mcp->txbuf, 18); + ret = mcp_send_data_req_status(mcp, mcp->txbuf, 18); mutex_unlock(&mcp->lock); + + return ret; } static int mcp_gpio_dir_set(struct mcp2221 *mcp, @@ -1206,7 +1208,7 @@ static int mcp2221_probe(struct hid_device *hdev, mcp->gc->direction_input = mcp_gpio_direction_input; mcp->gc->direction_output = mcp_gpio_direction_output; mcp->gc->get_direction = mcp_gpio_get_direction; - mcp->gc->set = mcp_gpio_set; + mcp->gc->set_rv = mcp_gpio_set; mcp->gc->get = mcp_gpio_get; mcp->gc->ngpio = MCP_NGPIO; mcp->gc->base = -1; -- cgit v1.2.3-59-g8ed1b