* [PATCH -next 06/19] HID: nintendo: Use devm_hid_hw_start_and_open in nintendo_hid_probe()
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the nintendo module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the err_close and err_stop lables.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hid/hid-nintendo.c | 23 ++++-------------------
1 file changed, 4 insertions(+), 19 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 58cd0506e431..45ac4fd3c7ea 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -2673,31 +2673,23 @@ static int nintendo_hid_probe(struct hid_device *hdev,
*/
hdev->version |= 0x8000;
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
- if (ret) {
- hid_err(hdev, "HW start failed\n");
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
+ if (ret)
goto err_wq;
- }
-
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "cannot start hardware I/O\n");
- goto err_stop;
- }
hid_device_io_start(hdev);
ret = joycon_init(hdev);
if (ret) {
hid_err(hdev, "Failed to initialize controller; ret=%d\n", ret);
- goto err_close;
+ goto err_wq;
}
/* Initialize the leds */
ret = joycon_leds_create(ctlr);
if (ret) {
hid_err(hdev, "Failed to create leds; ret=%d\n", ret);
- goto err_close;
+ goto err_wq;
}
/* Initialize the battery power supply */
@@ -2720,10 +2712,6 @@ static int nintendo_hid_probe(struct hid_device *hdev,
err_ida:
ida_free(&nintendo_player_id_allocator, ctlr->player_id);
-err_close:
- hid_hw_close(hdev);
-err_stop:
- hid_hw_stop(hdev);
err_wq:
destroy_workqueue(ctlr->rumble_queue);
err:
@@ -2745,9 +2733,6 @@ static void nintendo_hid_remove(struct hid_device *hdev)
destroy_workqueue(ctlr->rumble_queue);
ida_free(&nintendo_player_id_allocator, ctlr->player_id);
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
#ifdef CONFIG_PM
--
2.34.1
^ permalink raw reply related
* [PATCH -next 04/19] HID: mcp2200: Use devm_hid_hw_start_and_open in mcp2200_probe()
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the mcp2200 module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. So there is no need to close and
stop hid when an error occurs. At the same time, since there is no need to
do any operations in mcp2200_remove() now, so delete .remote operation.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hid/hid-mcp2200.c | 22 ++--------------------
1 file changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/hid/hid-mcp2200.c b/drivers/hid/hid-mcp2200.c
index bf57f7f6caa0..56d72fc5623d 100644
--- a/drivers/hid/hid-mcp2200.c
+++ b/drivers/hid/hid-mcp2200.c
@@ -329,22 +329,13 @@ static int mcp2200_probe(struct hid_device *hdev, const struct hid_device_id *id
return ret;
}
- ret = hid_hw_start(hdev, 0);
- if (ret) {
- hid_err(hdev, "can't start hardware\n");
+ ret = devm_hid_hw_start_and_open(hdev, 0);
+ if (ret)
return ret;
- }
hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n", hdev->version >> 8,
hdev->version & 0xff, hdev->name, hdev->phys);
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "can't open device\n");
- hid_hw_stop(hdev);
- return ret;
- }
-
mutex_init(&mcp->lock);
init_completion(&mcp->wait_in_report);
hid_set_drvdata(hdev, mcp);
@@ -356,20 +347,12 @@ static int mcp2200_probe(struct hid_device *hdev, const struct hid_device_id *id
ret = devm_gpiochip_add_data(&hdev->dev, &mcp->gc, mcp);
if (ret < 0) {
hid_err(hdev, "Unable to register gpiochip\n");
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
return ret;
}
return 0;
}
-static void mcp2200_remove(struct hid_device *hdev)
-{
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
-}
-
static const struct hid_device_id mcp2200_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_MCP2200) },
{ }
@@ -380,7 +363,6 @@ static struct hid_driver mcp2200_driver = {
.name = "mcp2200",
.id_table = mcp2200_devices,
.probe = mcp2200_probe,
- .remove = mcp2200_remove,
.raw_event = mcp2200_raw_event,
};
--
2.34.1
^ permalink raw reply related
* [PATCH -next 05/19] HID: mcp2221: Use devm_hid_hw_start_and_open in mcp2221_probe()
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the mcp2221 module use devm_add_action_or_reset() to manage
device resource for HID unregistration, now that a universal interface
has been provided, consider using a universal interface to replace it.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hid/hid-mcp2221.c | 26 ++------------------------
1 file changed, 2 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index 0f93c22a479f..3b8269f7e923 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -932,15 +932,6 @@ static int mcp2221_raw_event(struct hid_device *hdev,
return 1;
}
-/* Device resource managed function for HID unregistration */
-static void mcp2221_hid_unregister(void *ptr)
-{
- struct hid_device *hdev = ptr;
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
-}
-
/* This is needed to be sure hid_hw_stop() isn't called twice by the subsystem */
static void mcp2221_remove(struct hid_device *hdev)
{
@@ -1141,31 +1132,18 @@ static int mcp2221_probe(struct hid_device *hdev,
* This driver uses the .raw_event callback and therefore does not need any
* HID_CONNECT_xxx flags.
*/
- ret = hid_hw_start(hdev, 0);
- if (ret) {
- hid_err(hdev, "can't start hardware\n");
+ ret = devm_hid_hw_start_and_open(hdev, 0);
+ if (ret)
return ret;
- }
hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n", hdev->version >> 8,
hdev->version & 0xff, hdev->name, hdev->phys);
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "can't open device\n");
- hid_hw_stop(hdev);
- return ret;
- }
-
mutex_init(&mcp->lock);
init_completion(&mcp->wait_in_report);
hid_set_drvdata(hdev, mcp);
mcp->hdev = hdev;
- ret = devm_add_action_or_reset(&hdev->dev, mcp2221_hid_unregister, hdev);
- if (ret)
- return ret;
-
hid_device_io_start(hdev);
/* Set I2C bus clock diviser */
--
2.34.1
^ permalink raw reply related
* [PATCH -next 03/19] HID: ft260: Use devm_hid_hw_start_and_open in ft260_probe()
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the ft260 module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the err_hid_close, err_hid_stop
and err_i2c_free lables, and directly return the error code when an
error occurs.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hid/hid-ft260.c | 32 +++++++-------------------------
1 file changed, 7 insertions(+), 25 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 333341e80b0e..272165ebf46c 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -976,23 +976,15 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
- ret = hid_hw_start(hdev, 0);
- if (ret) {
- hid_err(hdev, "failed to start HID HW\n");
+ ret = devm_hid_hw_start_and_open(hdev, 0);
+ if (ret)
return ret;
- }
-
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "failed to open HID HW\n");
- goto err_hid_stop;
- }
ret = ft260_hid_feature_report_get(hdev, FT260_CHIP_VERSION,
(u8 *)&version, sizeof(version));
if (ret < 0) {
hid_err(hdev, "failed to retrieve chip version\n");
- goto err_hid_close;
+ return ret;
}
hid_info(hdev, "chip code: %02x%02x %02x%02x\n",
@@ -1001,7 +993,7 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = ft260_is_interface_enabled(hdev);
if (ret <= 0)
- goto err_hid_close;
+ return ret;
hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n",
hdev->version >> 8, hdev->version & 0xff, hdev->name,
@@ -1028,24 +1020,17 @@ static int ft260_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = i2c_add_adapter(&dev->adap);
if (ret) {
hid_err(hdev, "failed to add i2c adapter\n");
- goto err_hid_close;
+ return ret;
}
ret = sysfs_create_group(&hdev->dev.kobj, &ft260_attr_group);
if (ret < 0) {
hid_err(hdev, "failed to create sysfs attrs\n");
- goto err_i2c_free;
+ i2c_del_adapter(&dev->adap);
+ return ret;
}
return 0;
-
-err_i2c_free:
- i2c_del_adapter(&dev->adap);
-err_hid_close:
- hid_hw_close(hdev);
-err_hid_stop:
- hid_hw_stop(hdev);
- return ret;
}
static void ft260_remove(struct hid_device *hdev)
@@ -1057,9 +1042,6 @@ static void ft260_remove(struct hid_device *hdev)
sysfs_remove_group(&hdev->dev.kobj, &ft260_attr_group);
i2c_del_adapter(&dev->adap);
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
--
2.34.1
^ permalink raw reply related
* [PATCH -next 02/19] HID: cp2112: Use devm_hid_hw_start_and_open in cp2112_probe()
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
Currently, the cp2112 module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the err_hid_close and err_hid_stop
lables.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hid/hid-cp2112.c | 26 +++-----------------------
1 file changed, 3 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 20a0d1315d90..6d65c65f1b83 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1215,22 +1215,14 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
- if (ret) {
- hid_err(hdev, "hw start failed\n");
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
+ if (ret)
return ret;
- }
-
- ret = hid_hw_open(hdev);
- if (ret) {
- hid_err(hdev, "hw open failed\n");
- goto err_hid_stop;
- }
ret = hid_hw_power(hdev, PM_HINT_FULLON);
if (ret < 0) {
hid_err(hdev, "power management error: %d\n", ret);
- goto err_hid_close;
+ return ret;
}
ret = cp2112_hid_get(hdev, CP2112_GET_VERSION_INFO, buf, sizeof(buf),
@@ -1334,10 +1326,6 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
i2c_del_adapter(&dev->adap);
err_power_normal:
hid_hw_power(hdev, PM_HINT_NORMAL);
-err_hid_close:
- hid_hw_close(hdev);
-err_hid_stop:
- hid_hw_stop(hdev);
return ret;
}
@@ -1354,14 +1342,6 @@ static void cp2112_remove(struct hid_device *hdev)
}
gpiochip_remove(&dev->gc);
- /* i2c_del_adapter has finished removing all i2c devices from our
- * adapter. Well behaved devices should no longer call our cp2112_xfer
- * and should have waited for any pending calls to finish. It has also
- * waited for device_unregister(&adap->dev) to complete. Therefore we
- * can safely free our struct cp2112_device.
- */
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static int cp2112_raw_event(struct hid_device *hdev, struct hid_report *report,
--
2.34.1
^ permalink raw reply related
* [PATCH -next 01/19] HID: core: Use devm_add_action_or_reset helper to manage hid resources
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
In-Reply-To: <20240904123607.3407364-1-lizetao1@huawei.com>
By adding a custom action to the device, it can bind the hid resource
to the hid_device life cycle. The framework automatically close and stop
the hid resources before hid_device is released, and the users do not
need to pay attention to the timing of hid resource release.
Signed-off-by: Li Zetao <lizetao1@huawei.com>
---
drivers/hid/hid-core.c | 40 ++++++++++++++++++++++++++++++++++++++++
include/linux/hid.h | 2 ++
2 files changed, 42 insertions(+)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 30de92d0bf0f..71143c0a4a02 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2416,6 +2416,46 @@ void hid_hw_close(struct hid_device *hdev)
}
EXPORT_SYMBOL_GPL(hid_hw_close);
+static void hid_hw_close_and_stop(void *data)
+{
+ struct hid_device *hdev = data;
+
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+}
+
+/**
+ * devm_hid_hw_start_and_open - manage hid resources through custom action
+ *
+ * @hdev: hid device
+ * @connect_mask: which outputs to connect, see HID_CONNECT_*
+ *
+ * Bind the hid resource to the hid_device life cycle and register
+ * an action to release the hid resource. The users do not need to
+ * pay attention to the release of hid.
+ */
+
+int devm_hid_hw_start_and_open(struct hid_device *hdev, unsigned int connect_mask)
+{
+ int ret;
+
+ ret = hid_hw_start(hdev, connect_mask);
+ if (ret) {
+ hid_err(hdev, "hw start failed with %d\n", ret);
+ return ret;
+ }
+
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev, "hw open failed with %d\n", ret);
+ hid_hw_stop(hdev);
+ return ret;
+ }
+
+ return devm_add_action_or_reset(&hdev->dev, hid_hw_close_and_stop, hdev);
+}
+EXPORT_SYMBOL_GPL(devm_hid_hw_start_and_open);
+
/**
* hid_hw_request - send report request to device
*
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 121d5b8bc867..0ce217aa5f62 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1125,6 +1125,8 @@ int __must_check hid_hw_start(struct hid_device *hdev,
void hid_hw_stop(struct hid_device *hdev);
int __must_check hid_hw_open(struct hid_device *hdev);
void hid_hw_close(struct hid_device *hdev);
+int __must_check devm_hid_hw_start_and_open(struct hid_device *hdev,
+ unsigned int connect_mask);
void hid_hw_request(struct hid_device *hdev,
struct hid_report *report, enum hid_class_request reqtype);
int __hid_hw_raw_request(struct hid_device *hdev,
--
2.34.1
^ permalink raw reply related
* [PATCH -next 00/19] HID: convert to devm_hid_hw_start_and_open()
From: Li Zetao @ 2024-09-04 12:35 UTC (permalink / raw)
To: jikos, bentiss, michael.zaidman, gupt21, djogorchock, rrameshbabu,
bonbons, roderick.colenbrander, david, savicaleksa83, me,
jdelvare, linux, mail, wilken.gottwalt, jonas, mezin.alexander
Cc: lizetao1, linux-input, linux-i2c, linux-hwmon
Hi, all
This patchset is dedicated to using the life cycle approach to manage
hid resources. By keeping hid resources consistent with the life cycle
of the device, we ensure that resources are available during the life
cycle and the hid resources can be released before device release.
Going one step further, since the module does not need to recycle hid
resources by itself, the goto-release resource release coding can be
avoided. It also reduces the risk of resources not being released.
Li Zetao (19):
HID: core: Use devm_add_action_or_reset helper to manage hid resources
HID: cp2112: Use devm_hid_hw_start_and_open in cp2112_probe()
HID: ft260: Use devm_hid_hw_start_and_open in ft260_probe()
HID: mcp2200: Use devm_hid_hw_start_and_open in mcp2200_probe()
HID: mcp2221: Use devm_hid_hw_start_and_open in mcp2221_probe()
HID: nintendo: Use devm_hid_hw_start_and_open in nintendo_hid_probe()
HID: shield: Use devm_hid_hw_start_and_open in shield_probe()
HID: hid-picolcd: Use devm_hid_hw_start_and_open in picolcd_probe()
HID: playstation: Use devm_hid_hw_start_and_open in ps_probe()
HID: hid-steam: Use devm_hid_hw_start_and_open in steam_probe()
HID: wiimote: Use devm_hid_hw_start_and_open in wiimote_hid_probe()
hwmon: (aquacomputer_d5next) Use devm_hid_hw_start_and_open in
aqc_probe()
hwmon: Use devm_hid_hw_start_and_open in rog_ryujin_probe()
hwmon: (corsair-cpro) Use devm_hid_hw_start_and_open in ccp_probe()
hwmon: (corsair-psu) Use devm_hid_hw_start_and_open in
corsairpsu_probe()
hwmon: (gigabyte_waterforce) Use devm_hid_hw_start_and_open in
waterforce_probe()
hwmon: (nzxt-kraken2) Use devm_hid_hw_start_and_open in
kraken2_probe()
hwmon: (nzxt-kraken3) Use devm_hid_hw_start_and_open in
kraken3_probe()
hwmon: (nzxt-smart2) Use devm_hid_hw_start_and_open in
nzxt_smart2_hid_probe()
drivers/hid/hid-core.c | 40 +++++++++++++++++++++++++++++
drivers/hid/hid-cp2112.c | 26 +++----------------
drivers/hid/hid-ft260.c | 32 +++++------------------
drivers/hid/hid-mcp2200.c | 22 ++--------------
drivers/hid/hid-mcp2221.c | 26 ++-----------------
drivers/hid/hid-nintendo.c | 23 +++--------------
drivers/hid/hid-nvidia-shield.c | 20 +++------------
drivers/hid/hid-picolcd_core.c | 22 +++-------------
drivers/hid/hid-playstation.c | 27 +++----------------
drivers/hid/hid-steam.c | 18 ++-----------
drivers/hid/hid-wiimote-core.c | 20 +++------------
drivers/hwmon/aquacomputer_d5next.c | 39 +++++++---------------------
drivers/hwmon/asus_rog_ryujin.c | 29 ++++-----------------
drivers/hwmon/corsair-cpro.c | 24 ++++-------------
drivers/hwmon/corsair-psu.c | 24 ++++-------------
drivers/hwmon/gigabyte_waterforce.c | 29 ++++-----------------
drivers/hwmon/nzxt-kraken2.c | 23 +++--------------
drivers/hwmon/nzxt-kraken3.c | 34 +++++-------------------
drivers/hwmon/nzxt-smart2.c | 22 +++-------------
include/linux/hid.h | 2 ++
20 files changed, 118 insertions(+), 384 deletions(-)
--
2.34.1
^ permalink raw reply
* Re: [PATCH 13/22] Input: iqs269a - use cleanup facility for fwnodes
From: Javier Carrasco @ 2024-09-04 11:13 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel
In-Reply-To: <20240904044806.1047847-1-dmitry.torokhov@gmail.com>
On 04/09/2024 06:48, Dmitry Torokhov wrote:
> Use __free(fwnode_handle) cleanup facility to ensure that references to
> acquired fwnodes are dropped at appropriate times automatically.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/misc/iqs269a.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/input/misc/iqs269a.c b/drivers/input/misc/iqs269a.c
> index c34d847fa4af..1851848e2cd3 100644
> --- a/drivers/input/misc/iqs269a.c
> +++ b/drivers/input/misc/iqs269a.c
> @@ -550,7 +550,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
> const struct fwnode_handle *ch_node)
> {
> struct i2c_client *client = iqs269->client;
> - struct fwnode_handle *ev_node;
> struct iqs269_ch_reg *ch_reg;
> u16 engine_a, engine_b;
> unsigned int reg, val;
> @@ -727,8 +726,9 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
> }
>
> for (i = 0; i < ARRAY_SIZE(iqs269_events); i++) {
> - ev_node = fwnode_get_named_child_node(ch_node,
> - iqs269_events[i].name);
> + struct fwnode_handle *ev_node __free(fwnode_handle) =
> + fwnode_get_named_child_node(ch_node,
> + iqs269_events[i].name);
> if (!ev_node)
> continue;
>
> @@ -737,7 +737,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
> dev_err(&client->dev,
> "Invalid channel %u threshold: %u\n",
> reg, val);
> - fwnode_handle_put(ev_node);
> return -EINVAL;
> }
>
> @@ -751,7 +750,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
> dev_err(&client->dev,
> "Invalid channel %u hysteresis: %u\n",
> reg, val);
> - fwnode_handle_put(ev_node);
> return -EINVAL;
> }
>
> @@ -767,7 +765,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
> }
>
> error = fwnode_property_read_u32(ev_node, "linux,code", &val);
> - fwnode_handle_put(ev_node);
> if (error == -EINVAL) {
> continue;
> } else if (error) {
Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
^ permalink raw reply
* Re: [PATCH 14/22] Input: iqs626a - use cleanup facility for fwnodes
From: Javier Carrasco @ 2024-09-04 11:10 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel,
Javier Carrasco Cruz
In-Reply-To: <20240904044814.1048062-1-dmitry.torokhov@gmail.com>
On 04/09/2024 06:48, Dmitry Torokhov wrote:
> Use __free(fwnode_handle) cleanup facility to ensure that references to
> acquired fwnodes are dropped at appropriate times automatically.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/misc/iqs626a.c | 22 ++++++----------------
> 1 file changed, 6 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/input/misc/iqs626a.c b/drivers/input/misc/iqs626a.c
> index 0dab54d3a060..7a6e6927f331 100644
> --- a/drivers/input/misc/iqs626a.c
> +++ b/drivers/input/misc/iqs626a.c
> @@ -462,7 +462,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> {
> struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> struct i2c_client *client = iqs626->client;
> - struct fwnode_handle *ev_node;
> const char *ev_name;
> u8 *thresh, *hyst;
> unsigned int val;
> @@ -501,6 +500,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> if (!iqs626_channels[ch_id].events[i])
> continue;
>
> + struct fwnode_handle *ev_node __free(fwnode_handle) = NULL;
> if (ch_id == IQS626_CH_TP_2 || ch_id == IQS626_CH_TP_3) {
> /*
> * Trackpad touch events are simply described under the
> @@ -530,7 +530,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid input type: %u\n",
> val);
> - fwnode_handle_put(ev_node);
> return -EINVAL;
> }
>
> @@ -545,7 +544,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid %s channel hysteresis: %u\n",
> fwnode_get_name(ch_node), val);
> - fwnode_handle_put(ev_node);
> return -EINVAL;
> }
>
> @@ -566,7 +564,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid %s channel threshold: %u\n",
> fwnode_get_name(ch_node), val);
> - fwnode_handle_put(ev_node);
> return -EINVAL;
> }
>
> @@ -575,8 +572,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
> else
> *(thresh + iqs626_events[i].th_offs) = val;
> }
> -
> - fwnode_handle_put(ev_node);
> }
>
> return 0;
> @@ -774,12 +769,12 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> for (i = 0; i < iqs626_channels[ch_id].num_ch; i++) {
> u8 *ati_base = &sys_reg->tp_grp_reg.ch_reg_tp[i].ati_base;
> u8 *thresh = &sys_reg->tp_grp_reg.ch_reg_tp[i].thresh;
> - struct fwnode_handle *tc_node;
> char tc_name[10];
>
> snprintf(tc_name, sizeof(tc_name), "channel-%d", i);
>
> - tc_node = fwnode_get_named_child_node(ch_node, tc_name);
> + struct fwnode_handle *tc_node __free(fwnode_handle) =
> + fwnode_get_named_child_node(ch_node, tc_name);
> if (!tc_node)
> continue;
>
> @@ -790,7 +785,6 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid %s %s ATI base: %u\n",
> fwnode_get_name(ch_node), tc_name, val);
> - fwnode_handle_put(tc_node);
> return -EINVAL;
> }
>
> @@ -803,14 +797,11 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
> dev_err(&client->dev,
> "Invalid %s %s threshold: %u\n",
> fwnode_get_name(ch_node), tc_name, val);
> - fwnode_handle_put(tc_node);
> return -EINVAL;
> }
>
> *thresh = val;
> }
> -
> - fwnode_handle_put(tc_node);
> }
>
> if (!fwnode_property_present(ch_node, "linux,keycodes"))
> @@ -1233,7 +1224,6 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
> {
> struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
> struct i2c_client *client = iqs626->client;
> - struct fwnode_handle *ch_node;
> unsigned int val;
> int error, i;
> u16 general;
> @@ -1375,13 +1365,13 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
> sys_reg->active = 0;
>
> for (i = 0; i < ARRAY_SIZE(iqs626_channels); i++) {
> - ch_node = device_get_named_child_node(&client->dev,
> - iqs626_channels[i].name);
> + struct fwnode_handle *ch_node __free(fwnode_handle) =
> + device_get_named_child_node(&client->dev,
> + iqs626_channels[i].name);
> if (!ch_node)
> continue;
>
> error = iqs626_parse_channel(iqs626, ch_node, i);
> - fwnode_handle_put(ch_node);
> if (error)
> return error;
>
Reviewed-by: Javier Carrasco <javier.carrasco.cruz@gmail.com>
^ permalink raw reply
* Re: [PATCH 15/22] Input: iqs7222 - use cleanup facility for fwnodes
From: Javier Carrasco @ 2024-09-04 10:50 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Jeff LaBundy, linux-kernel
In-Reply-To: <20240904044825.1048256-1-dmitry.torokhov@gmail.com>
Hi Dmitry,
On 04/09/2024 06:48, Dmitry Torokhov wrote:
> Use __free(fwnode_handle) cleanup facility to ensure that references to
> acquired fwnodes are dropped at appropriate times automatically.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/misc/iqs7222.c | 30 ++++++++++++++----------------
> 1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
> index 9ca5a743f19f..d9b87606ff7a 100644
> --- a/drivers/input/misc/iqs7222.c
> +++ b/drivers/input/misc/iqs7222.c
...
> @@ -2818,9 +2813,9 @@ static int iqs7222_parse_reg_grp(struct iqs7222_private *iqs7222,
> int reg_grp_index)
> {
> struct i2c_client *client = iqs7222->client;
> - struct fwnode_handle *reg_grp_node;
> int error;
>
Nit: reg_grp_node could stay at the top (where it used to be), as you
are assigning it to NULL because there are no sensible assignments at
this point.
> + struct fwnode_handle *reg_grp_node __free(fwnode_handle) = NULL;
> if (iqs7222_reg_grp_names[reg_grp]) {
> char reg_grp_name[16];
>
Best regards,
Javier Carrasco
^ permalink raw reply
* Re: [PATCH HID 4/7] HID: bpf: allow BPF programs to force using hid-generic
From: Benjamin Tissoires @ 2024-09-04 9:28 UTC (permalink / raw)
To: Peter Hutterer
Cc: Jiri Kosina, Vicki Pfau, Shuah Khan, Jonathan Corbet, linux-input,
linux-kselftest, linux-kernel, bpf, linux-doc
In-Reply-To: <whzoigqsbgmhxhk7qws4o63l23a5ravmpqyro4kvajomypljgf@x5pakwey5wej>
On Sep 03 2024, Benjamin Tissoires wrote:
> On Sep 03 2024, Peter Hutterer wrote:
> > On Tue, Sep 03, 2024 at 01:14:34AM +0900, Benjamin Tissoires wrote:
> > > The use case is when we fix a device through HID-BPF, 99% of the cases
> > > we want the device to use hid-generic now instead of a dedicated device.
> >
> > s/dedicated device/dedicated driver/ in the commit message
> >
> > > That's because the dedicated device might also want to change the report
> > > descriptor, or will be handling the device in a different way the new
> > > fixed device is using.
> > >
> > > In hid-core, after matching for the device (so that we only call this new
> > > hook on compatible drivers), we call for `.hid_bpf_driver_probe`.
> > > The function can not communicate with the device because it is not yet
> > > started, but it can make educated guesses and decide to:
> > > - let hid-core decide by itself
> > > - force the use of this driver (by comparing the provided name with
> > > "hid-generic" for instance)
> > > - force hid-core to ignore this driver for this device.
> > >
> > > For API stability, we don't rely on a bitfield or a return value for
> > > chosing hid-core behavior. We simply have a couple of writeable fields
> > > in the new struct hid_bpf_driver, and then hid-core can make its educated
> > > decision.
> > >
> > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > > ---
> > > Documentation/hid/hid-bpf.rst | 2 +-
> > > drivers/hid/bpf/hid_bpf_dispatch.c | 31 ++++++++++++++++++++++++++++
> > > drivers/hid/bpf/hid_bpf_struct_ops.c | 3 +++
> > > drivers/hid/hid-core.c | 6 ++++++
> > > include/linux/hid_bpf.h | 40 ++++++++++++++++++++++++++++++++++++
> > > 5 files changed, 81 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/hid/hid-bpf.rst b/Documentation/hid/hid-bpf.rst
> > > index 5939eeafb361..05a43f11cdab 100644
> > > --- a/Documentation/hid/hid-bpf.rst
> > > +++ b/Documentation/hid/hid-bpf.rst
> > > @@ -190,7 +190,7 @@ User API data structures available in programs:
> > > -----------------------------------------------
> > >
> > > .. kernel-doc:: include/linux/hid_bpf.h
> > > - :identifiers: hid_bpf_ctx
> > > + :identifiers: hid_bpf_ctx hid_bpf_driver
> > >
> > > Available API that can be used in all HID-BPF struct_ops programs:
> > > ------------------------------------------------------------------
> > > diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c
> > > index a272a086c950..2df136d64152 100644
> > > --- a/drivers/hid/bpf/hid_bpf_dispatch.c
> > > +++ b/drivers/hid/bpf/hid_bpf_dispatch.c
> > > @@ -189,6 +189,37 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *s
> > > }
> > > EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup);
> > >
> > > +int call_hid_bpf_driver_probe(struct hid_device *hdev, struct hid_driver *hdrv,
> > > + const struct hid_device_id *id)
> > > +{
> > > + struct hid_bpf_driver drv = { 0 };
> > > + struct hid_bpf_ops *e;
> > > + int idx;
> > +
> > > + if (strscpy(drv.name, hdrv->name, sizeof(drv.name)) < 0)
> > > + return 0;
> > > +
> > > + idx = srcu_read_lock(&hdev->bpf.srcu);
> > > + list_for_each_entry_srcu(e, &hdev->bpf.prog_list, list,
> > > + srcu_read_lock_held(&hdev->bpf.srcu)) {
> > > + if (!e->hid_driver_probe)
> > > + continue;
> > > +
> > > + e->hid_driver_probe(hdev, &drv, id);
> > > + }
> > > +
> > > + srcu_read_unlock(&hdev->bpf.srcu, idx);
> > > +
> > > + if (drv.force_driver)
> > > + return 1;
> > > +
> > > + if (drv.ignore_driver)
> > > + return -1;
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(call_hid_bpf_driver_probe);
> > > +
> > > static int device_match_id(struct device *dev, const void *id)
> > > {
> > > struct hid_device *hdev = to_hid_device(dev);
> > > diff --git a/drivers/hid/bpf/hid_bpf_struct_ops.c b/drivers/hid/bpf/hid_bpf_struct_ops.c
> > > index cd696c59ba0f..1e13a22f73a1 100644
> > > --- a/drivers/hid/bpf/hid_bpf_struct_ops.c
> > > +++ b/drivers/hid/bpf/hid_bpf_struct_ops.c
> > > @@ -46,6 +46,7 @@ static int hid_bpf_ops_check_member(const struct btf_type *t,
> > > case offsetof(struct hid_bpf_ops, hid_rdesc_fixup):
> > > case offsetof(struct hid_bpf_ops, hid_hw_request):
> > > case offsetof(struct hid_bpf_ops, hid_hw_output_report):
> > > + case offsetof(struct hid_bpf_ops, hid_driver_probe):
> > > break;
> > > default:
> > > if (prog->sleepable)
> > > @@ -79,6 +80,8 @@ static int hid_bpf_ops_btf_struct_access(struct bpf_verifier_log *log,
> > > WRITE_RANGE(hid_device, name, true),
> > > WRITE_RANGE(hid_device, uniq, true),
> > > WRITE_RANGE(hid_device, phys, true),
> > > + WRITE_RANGE(hid_bpf_driver, force_driver, false),
> > > + WRITE_RANGE(hid_bpf_driver, ignore_driver, false),
> > > };
> > > #undef WRITE_RANGE
> > > const struct btf_type *state = NULL;
> > > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > > index 988d0acbdf04..7845f0a789ec 100644
> > > --- a/drivers/hid/hid-core.c
> > > +++ b/drivers/hid/hid-core.c
> > > @@ -2639,10 +2639,16 @@ static bool hid_check_device_match(struct hid_device *hdev,
> > > struct hid_driver *hdrv,
> > > const struct hid_device_id **id)
> > > {
> > > + int ret;
> > > +
> > > *id = hid_match_device(hdev, hdrv);
> > > if (!*id)
> > > return false;
> > >
> > > + ret = call_hid_bpf_driver_probe(hdev, hdrv, *id);
> > > + if (ret)
> > > + return ret > 0;
> > > +
> > > if (hdrv->match)
> > > return hdrv->match(hdev, hid_ignore_special_drivers);
> > >
> > > diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
> > > index d4d063cf63b5..20693c218857 100644
> > > --- a/include/linux/hid_bpf.h
> > > +++ b/include/linux/hid_bpf.h
> > > @@ -9,6 +9,7 @@
> > > #include <uapi/linux/hid.h>
> > >
> > > struct hid_device;
> > > +struct hid_driver;
> > >
> > > /*
> > > * The following is the user facing HID BPF API.
> > > @@ -80,6 +81,22 @@ struct hid_ops {
> > >
> > > extern struct hid_ops *hid_ops;
> > >
> > > +/**
> > > + * struct hid_bpf_driver - User accessible data for the ``hid_bpf_probe``
> > > + * struct_ops
> > > + *
> > > + * @name: the name of the driver currently being treated
> > > + * @force_driver: set this to ``true`` to force hid-core to use this driver,
> > > + * bypassing any further decision made by this driver
> > > + * @ignore_driver: set this to ``true`` to force hid-core to ignore this driver,
> > > + * bypassing any further decision made by this driver
> >
> > If I set both to false or true, what happens? The two seem to be
>
> force_driver has priority over ignore_driver.
>
> > mutually exclusive, in userspace I'd use an enum here to have a
> > NOOP/FORCE_DRIVER/IGNORE_DRIVER value range (that can be extended later).
> > Maybe something like that is an option?
>
> enum also has the advantage to be exported in vmlinux.h.
>
> FWIW, the idea behind adding new fields in a struct was to get the
> backward compatibility for free. Because the verifier/relocator will see
> if we are using the correct field entries.
>
> OTOH, maybe we can make the function return the afformended enum, and
> drop those two fields.
>
> I think we should probably abort processing of any bpf sets the return
> value to anything else than NOOP.
>
> I'll work a little bit more on that.
>
> Cheers,
> Benjamin
>
> >
> > > + */
> > > +struct hid_bpf_driver {
> > > + __u8 name[64];
> > > + bool force_driver;
> > > + bool ignore_driver;
> > > +};
> > > +
> > > /**
> > > * struct hid_bpf_ops - A BPF struct_ops of callbacks allowing to attach HID-BPF
> > > * programs to a HID device
> > > @@ -178,6 +195,25 @@ struct hid_bpf_ops {
> > > */
> > > int (*hid_hw_output_report)(struct hid_bpf_ctx *ctx, u64 source);
> > >
> > > + /**
> > > + * @hid_driver_probe: called before the kernel ``.probe()`` function
> > > + *
> > > + * It has the following arguments:
> > > + *
> > > + * ``hdev``: The HID device kernel representation
> > > + *
> > > + * ``hdrv``: A BPF partially writeable representation of a HID driver
> > > + *
> > > + * ``id``: The device match structure found in the driver
> > > + *
> > > + * Note that the device has not been started yet, and thus kfuncs like
> > > + * ``hid_hw_output_report`` will likely fail.
> >
> > Just to confirm, I can access the device's report descriptor though? For
I forgot to reply to this comment:
no, you don't :)
> > the devices that we're looking at (e.g. the foot pedals pretending to be
> > an apple keyboard) the driver name and what we can set in HID_BPF_CONFIG
> > are not going to be enough, we'll have to check the rdesc too.
You can check this in the probe syscall before unbinding/rebinding the
device. The device is uniquely linked to the bpf program you loaded, so
in theory this is sufficient.
Cheers,
Benjamin
> >
> > Cheers,
> > Peter
> >
> > > + *
> > > + * This function is useful to force/ignore a given supported HID driver,
> > > + * by writing ``true`` in ``hdrv->force_driver`` or ``hdrv->ignore_driver``
> > > + */
> > > + void (*hid_driver_probe)(struct hid_device *hdev, struct hid_bpf_driver *hdrv,
> > > + const struct hid_device_id *id);
> > >
> > > /* private: do not show up in the docs */
> > > struct hid_device *hdev;
> > > @@ -213,6 +249,8 @@ void hid_bpf_disconnect_device(struct hid_device *hdev);
> > > void hid_bpf_destroy_device(struct hid_device *hid);
> > > int hid_bpf_device_init(struct hid_device *hid);
> > > u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, u8 *rdesc, unsigned int *size);
> > > +int call_hid_bpf_driver_probe(struct hid_device *hdev, struct hid_driver *hdrv,
> > > + const struct hid_device_id *id);
> > > #else /* CONFIG_HID_BPF */
> > > static inline u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type,
> > > u8 *data, u32 *size, int interrupt,
> > > @@ -228,6 +266,8 @@ static inline int hid_bpf_connect_device(struct hid_device *hdev) { return 0; }
> > > static inline void hid_bpf_disconnect_device(struct hid_device *hdev) {}
> > > static inline void hid_bpf_destroy_device(struct hid_device *hid) {}
> > > static inline int hid_bpf_device_init(struct hid_device *hid) { return 0; }
> > > +static inline int call_hid_bpf_driver_probe(struct hid_device *hdev, struct hid_driver *hdrv,
> > > + const struct hid_device_id *id) { return 0; }
> > > /*
> > > * This specialized allocator has to be a macro for its allocations to be
> > > * accounted separately (to have a separate alloc_tag). The typecast is
> > >
> > > --
> > > 2.46.0
> > >
^ permalink raw reply
* [PATCH] HID: samples: Delete some unuseful comments
From: Liu Jing @ 2024-09-03 21:43 UTC (permalink / raw)
To: jikos; +Cc: bentiss, linux-input, linux-kernel, Liu Jing
Signed-off-by: Liu Jing <liujing@cmss.chinamobile.com>
diff --git a/samples/hid/hid_surface_dial.bpf.c b/samples/hid/hid_surface_dial.bpf.c
index 527d584812ab..dfb0383eb281 100644
--- a/samples/hid/hid_surface_dial.bpf.c
+++ b/samples/hid/hid_surface_dial.bpf.c
@@ -78,13 +78,7 @@ int set_haptic(struct haptic_syscall_args *args)
/* whenever resolution multiplier is not 3600, we have the fixed report descriptor */
res = (u16 *)&haptic_data[1];
if (*res != 3600) {
-// haptic_data[1] = 72; /* resolution multiplier */
-// haptic_data[2] = 0; /* resolution multiplier */
-// haptic_data[3] = 0; /* Repeat Count */
haptic_data[4] = 3; /* haptic Auto Trigger */
-// haptic_data[5] = 5; /* Waveform Cutoff Time */
-// haptic_data[6] = 80; /* Retrigger Period */
-// haptic_data[7] = 0; /* Retrigger Period */
} else {
haptic_data[4] = 0;
}
--
2.33.0
^ permalink raw reply related
* [PATCH 22/22] Input: sparcspkr - use guard notation when acquiring spinlock
From: Dmitry Torokhov @ 2024-09-04 4:49 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that locks are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/sparcspkr.c | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/input/misc/sparcspkr.c b/drivers/input/misc/sparcspkr.c
index 20020cbc0752..5de59ae90c67 100644
--- a/drivers/input/misc/sparcspkr.c
+++ b/drivers/input/misc/sparcspkr.c
@@ -69,7 +69,6 @@ static int bbc_spkr_event(struct input_dev *dev, unsigned int type, unsigned int
struct sparcspkr_state *state = dev_get_drvdata(dev->dev.parent);
struct bbc_beep_info *info = &state->u.bbc;
unsigned int count = 0;
- unsigned long flags;
if (type != EV_SND)
return -1;
@@ -85,7 +84,7 @@ static int bbc_spkr_event(struct input_dev *dev, unsigned int type, unsigned int
count = bbc_count_to_reg(info, count);
- spin_lock_irqsave(&state->lock, flags);
+ guard(spinlock_irqsave)(&state->lock);
if (count) {
sbus_writeb(0x01, info->regs + 0);
@@ -97,8 +96,6 @@ static int bbc_spkr_event(struct input_dev *dev, unsigned int type, unsigned int
sbus_writeb(0x00, info->regs + 0);
}
- spin_unlock_irqrestore(&state->lock, flags);
-
return 0;
}
@@ -107,7 +104,6 @@ static int grover_spkr_event(struct input_dev *dev, unsigned int type, unsigned
struct sparcspkr_state *state = dev_get_drvdata(dev->dev.parent);
struct grover_beep_info *info = &state->u.grover;
unsigned int count = 0;
- unsigned long flags;
if (type != EV_SND)
return -1;
@@ -121,7 +117,7 @@ static int grover_spkr_event(struct input_dev *dev, unsigned int type, unsigned
if (value > 20 && value < 32767)
count = 1193182 / value;
- spin_lock_irqsave(&state->lock, flags);
+ guard(spinlock_irqsave)(&state->lock);
if (count) {
/* enable counter 2 */
@@ -136,8 +132,6 @@ static int grover_spkr_event(struct input_dev *dev, unsigned int type, unsigned
sbus_writeb(sbus_readb(info->enable_reg) & 0xFC, info->enable_reg);
}
- spin_unlock_irqrestore(&state->lock, flags);
-
return 0;
}
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 21/22] Input: rotary_encoder - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:49 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/rotary_encoder.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index 6628fe540834..52761da9f999 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -113,7 +113,7 @@ static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
struct rotary_encoder *encoder = dev_id;
unsigned int state;
- mutex_lock(&encoder->access_mutex);
+ guard(mutex)(&encoder->access_mutex);
state = rotary_encoder_get_state(encoder);
@@ -136,8 +136,6 @@ static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
break;
}
- mutex_unlock(&encoder->access_mutex);
-
return IRQ_HANDLED;
}
@@ -146,7 +144,7 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
struct rotary_encoder *encoder = dev_id;
unsigned int state;
- mutex_lock(&encoder->access_mutex);
+ guard(mutex)(&encoder->access_mutex);
state = rotary_encoder_get_state(encoder);
@@ -159,8 +157,6 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
}
}
- mutex_unlock(&encoder->access_mutex);
-
return IRQ_HANDLED;
}
@@ -169,22 +165,19 @@ static irqreturn_t rotary_encoder_quarter_period_irq(int irq, void *dev_id)
struct rotary_encoder *encoder = dev_id;
unsigned int state;
- mutex_lock(&encoder->access_mutex);
+ guard(mutex)(&encoder->access_mutex);
state = rotary_encoder_get_state(encoder);
- if ((encoder->last_stable + 1) % 4 == state)
+ if ((encoder->last_stable + 1) % 4 == state) {
encoder->dir = 1;
- else if (encoder->last_stable == (state + 1) % 4)
+ rotary_encoder_report_event(encoder);
+ } else if (encoder->last_stable == (state + 1) % 4) {
encoder->dir = -1;
- else
- goto out;
-
- rotary_encoder_report_event(encoder);
+ rotary_encoder_report_event(encoder);
+ }
-out:
encoder->last_stable = state;
- mutex_unlock(&encoder->access_mutex);
return IRQ_HANDLED;
}
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 20/22] Input: regulator-haptic - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:49 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/regulator-haptic.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
index 02f73b7c0462..41af6aefaa07 100644
--- a/drivers/input/misc/regulator-haptic.c
+++ b/drivers/input/misc/regulator-haptic.c
@@ -83,12 +83,10 @@ static void regulator_haptic_work(struct work_struct *work)
struct regulator_haptic *haptic = container_of(work,
struct regulator_haptic, work);
- mutex_lock(&haptic->mutex);
+ guard(mutex)(&haptic->mutex);
if (!haptic->suspended)
regulator_haptic_set_voltage(haptic, haptic->magnitude);
-
- mutex_unlock(&haptic->mutex);
}
static int regulator_haptic_play_effect(struct input_dev *input, void *data,
@@ -207,17 +205,14 @@ static int regulator_haptic_suspend(struct device *dev)
struct regulator_haptic *haptic = platform_get_drvdata(pdev);
int error;
- error = mutex_lock_interruptible(&haptic->mutex);
- if (error)
- return error;
-
- regulator_haptic_set_voltage(haptic, 0);
-
- haptic->suspended = true;
+ scoped_guard(mutex_intr, &haptic->mutex) {
+ regulator_haptic_set_voltage(haptic, 0);
+ haptic->suspended = true;
- mutex_unlock(&haptic->mutex);
+ return 0;
+ }
- return 0;
+ return -EINTR;
}
static int regulator_haptic_resume(struct device *dev)
@@ -226,7 +221,7 @@ static int regulator_haptic_resume(struct device *dev)
struct regulator_haptic *haptic = platform_get_drvdata(pdev);
unsigned int magnitude;
- mutex_lock(&haptic->mutex);
+ guard(mutex)(&haptic->mutex);
haptic->suspended = false;
@@ -234,8 +229,6 @@ static int regulator_haptic_resume(struct device *dev)
if (magnitude)
regulator_haptic_set_voltage(haptic, magnitude);
- mutex_unlock(&haptic->mutex);
-
return 0;
}
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 19/22] Input: pwm-beeper - use guard notation when acquiring spinlock
From: Dmitry Torokhov @ 2024-09-04 4:49 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that locks are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/pwm-beeper.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/input/misc/pwm-beeper.c b/drivers/input/misc/pwm-beeper.c
index 5b9aedf4362f..0e19e97d98ec 100644
--- a/drivers/input/misc/pwm-beeper.c
+++ b/drivers/input/misc/pwm-beeper.c
@@ -203,9 +203,9 @@ static int pwm_beeper_suspend(struct device *dev)
* beeper->suspended, but to ensure that pwm_beeper_event
* does not re-submit work once flag is set.
*/
- spin_lock_irq(&beeper->input->event_lock);
- beeper->suspended = true;
- spin_unlock_irq(&beeper->input->event_lock);
+ scoped_guard(spinlock_irq, &beeper->input->event_lock) {
+ beeper->suspended = true;
+ }
pwm_beeper_stop(beeper);
@@ -216,9 +216,9 @@ static int pwm_beeper_resume(struct device *dev)
{
struct pwm_beeper *beeper = dev_get_drvdata(dev);
- spin_lock_irq(&beeper->input->event_lock);
- beeper->suspended = false;
- spin_unlock_irq(&beeper->input->event_lock);
+ scoped_guard(spinlock_irq, &beeper->input->event_lock) {
+ beeper->suspended = false;
+ }
/* Let worker figure out if we should resume beeping */
schedule_work(&beeper->work);
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 18/22] Input: powermate - use guard notation when acquiring spinlock
From: Dmitry Torokhov @ 2024-09-04 4:49 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that locks are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/powermate.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/input/misc/powermate.c b/drivers/input/misc/powermate.c
index 4b039abffc4b..ecb92ee5ebbc 100644
--- a/drivers/input/misc/powermate.c
+++ b/drivers/input/misc/powermate.c
@@ -194,22 +194,18 @@ static void powermate_sync_state(struct powermate_device *pm)
static void powermate_config_complete(struct urb *urb)
{
struct powermate_device *pm = urb->context;
- unsigned long flags;
if (urb->status)
printk(KERN_ERR "powermate: config urb returned %d\n", urb->status);
- spin_lock_irqsave(&pm->lock, flags);
+ guard(spinlock_irqsave)(&pm->lock);
powermate_sync_state(pm);
- spin_unlock_irqrestore(&pm->lock, flags);
}
/* Set the LED up as described and begin the sync with the hardware if required */
static void powermate_pulse_led(struct powermate_device *pm, int static_brightness, int pulse_speed,
int pulse_table, int pulse_asleep, int pulse_awake)
{
- unsigned long flags;
-
if (pulse_speed < 0)
pulse_speed = 0;
if (pulse_table < 0)
@@ -222,8 +218,7 @@ static void powermate_pulse_led(struct powermate_device *pm, int static_brightne
pulse_asleep = !!pulse_asleep;
pulse_awake = !!pulse_awake;
-
- spin_lock_irqsave(&pm->lock, flags);
+ guard(spinlock_irqsave)(&pm->lock);
/* mark state updates which are required */
if (static_brightness != pm->static_brightness) {
@@ -245,8 +240,6 @@ static void powermate_pulse_led(struct powermate_device *pm, int static_brightne
}
powermate_sync_state(pm);
-
- spin_unlock_irqrestore(&pm->lock, flags);
}
/* Callback from the Input layer when an event arrives from userspace to configure the LED */
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 17/22] Input: pegasus_notetaker - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:48 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/tablet/pegasus_notetaker.c | 86 +++++++++++++-----------
1 file changed, 48 insertions(+), 38 deletions(-)
diff --git a/drivers/input/tablet/pegasus_notetaker.c b/drivers/input/tablet/pegasus_notetaker.c
index a68da2988f9c..e1dc8365bfe9 100644
--- a/drivers/input/tablet/pegasus_notetaker.c
+++ b/drivers/input/tablet/pegasus_notetaker.c
@@ -214,6 +214,28 @@ static void pegasus_init(struct work_struct *work)
error);
}
+static int __pegasus_open(struct pegasus *pegasus)
+{
+ int error;
+
+ guard(mutex)(&pegasus->pm_mutex);
+
+ pegasus->irq->dev = pegasus->usbdev;
+ if (usb_submit_urb(pegasus->irq, GFP_KERNEL))
+ return -EIO;
+
+ error = pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE);
+ if (error) {
+ usb_kill_urb(pegasus->irq);
+ cancel_work_sync(&pegasus->init);
+ return error;
+ }
+
+ pegasus->is_open = true;
+ return 0;
+}
+
+
static int pegasus_open(struct input_dev *dev)
{
struct pegasus *pegasus = input_get_drvdata(dev);
@@ -223,39 +245,25 @@ static int pegasus_open(struct input_dev *dev)
if (error)
return error;
- mutex_lock(&pegasus->pm_mutex);
- pegasus->irq->dev = pegasus->usbdev;
- if (usb_submit_urb(pegasus->irq, GFP_KERNEL)) {
- error = -EIO;
- goto err_autopm_put;
+ error = __pegasus_open(pegasus);
+ if (error) {
+ usb_autopm_put_interface(pegasus->intf);
+ return error;
}
- error = pegasus_set_mode(pegasus, PEN_MODE_XY, NOTETAKER_LED_MOUSE);
- if (error)
- goto err_kill_urb;
-
- pegasus->is_open = true;
- mutex_unlock(&pegasus->pm_mutex);
return 0;
-
-err_kill_urb:
- usb_kill_urb(pegasus->irq);
- cancel_work_sync(&pegasus->init);
-err_autopm_put:
- mutex_unlock(&pegasus->pm_mutex);
- usb_autopm_put_interface(pegasus->intf);
- return error;
}
static void pegasus_close(struct input_dev *dev)
{
struct pegasus *pegasus = input_get_drvdata(dev);
- mutex_lock(&pegasus->pm_mutex);
- usb_kill_urb(pegasus->irq);
- cancel_work_sync(&pegasus->init);
- pegasus->is_open = false;
- mutex_unlock(&pegasus->pm_mutex);
+ scoped_guard(mutex, &pegasus->pm_mutex) {
+ usb_kill_urb(pegasus->irq);
+ cancel_work_sync(&pegasus->init);
+
+ pegasus->is_open = false;
+ }
usb_autopm_put_interface(pegasus->intf);
}
@@ -411,10 +419,10 @@ static int pegasus_suspend(struct usb_interface *intf, pm_message_t message)
{
struct pegasus *pegasus = usb_get_intfdata(intf);
- mutex_lock(&pegasus->pm_mutex);
+ guard(mutex)(&pegasus->pm_mutex);
+
usb_kill_urb(pegasus->irq);
cancel_work_sync(&pegasus->init);
- mutex_unlock(&pegasus->pm_mutex);
return 0;
}
@@ -422,31 +430,33 @@ static int pegasus_suspend(struct usb_interface *intf, pm_message_t message)
static int pegasus_resume(struct usb_interface *intf)
{
struct pegasus *pegasus = usb_get_intfdata(intf);
- int retval = 0;
- mutex_lock(&pegasus->pm_mutex);
+ guard(mutex)(&pegasus->pm_mutex);
+
if (pegasus->is_open && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
- retval = -EIO;
- mutex_unlock(&pegasus->pm_mutex);
+ return -EIO;
- return retval;
+ return 0;
}
static int pegasus_reset_resume(struct usb_interface *intf)
{
struct pegasus *pegasus = usb_get_intfdata(intf);
- int retval = 0;
+ int error;
+
+ guard(mutex)(&pegasus->pm_mutex);
- mutex_lock(&pegasus->pm_mutex);
if (pegasus->is_open) {
- retval = pegasus_set_mode(pegasus, PEN_MODE_XY,
+ error = pegasus_set_mode(pegasus, PEN_MODE_XY,
NOTETAKER_LED_MOUSE);
- if (!retval && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
- retval = -EIO;
+ if (error)
+ return error;
+
+ if (usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
+ return -EIO;
}
- mutex_unlock(&pegasus->pm_mutex);
- return retval;
+ return 0;
}
static const struct usb_device_id pegasus_ids[] = {
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 16/22] Input: max8997_haptic - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:48 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/max8997_haptic.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/input/misc/max8997_haptic.c b/drivers/input/misc/max8997_haptic.c
index 11cac4b7dddc..2853455daef2 100644
--- a/drivers/input/misc/max8997_haptic.c
+++ b/drivers/input/misc/max8997_haptic.c
@@ -153,19 +153,19 @@ static void max8997_haptic_enable(struct max8997_haptic *chip)
{
int error;
- mutex_lock(&chip->mutex);
+ guard(mutex)(&chip->mutex);
error = max8997_haptic_set_duty_cycle(chip);
if (error) {
dev_err(chip->dev, "set_pwm_cycle failed, error: %d\n", error);
- goto out;
+ return;
}
if (!chip->enabled) {
error = regulator_enable(chip->regulator);
if (error) {
dev_err(chip->dev, "Failed to enable regulator\n");
- goto out;
+ return;
}
max8997_haptic_configure(chip);
if (chip->mode == MAX8997_EXTERNAL_MODE) {
@@ -173,19 +173,16 @@ static void max8997_haptic_enable(struct max8997_haptic *chip)
if (error) {
dev_err(chip->dev, "Failed to enable PWM\n");
regulator_disable(chip->regulator);
- goto out;
+ return;
}
}
chip->enabled = true;
}
-
-out:
- mutex_unlock(&chip->mutex);
}
static void max8997_haptic_disable(struct max8997_haptic *chip)
{
- mutex_lock(&chip->mutex);
+ guard(mutex)(&chip->mutex);
if (chip->enabled) {
chip->enabled = false;
@@ -194,8 +191,6 @@ static void max8997_haptic_disable(struct max8997_haptic *chip)
pwm_disable(chip->pwm);
regulator_disable(chip->regulator);
}
-
- mutex_unlock(&chip->mutex);
}
static void max8997_haptic_play_effect_work(struct work_struct *work)
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 15/22] Input: iqs7222 - use cleanup facility for fwnodes
From: Dmitry Torokhov @ 2024-09-04 4:48 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Use __free(fwnode_handle) cleanup facility to ensure that references to
acquired fwnodes are dropped at appropriate times automatically.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/iqs7222.c | 30 ++++++++++++++----------------
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
index 9ca5a743f19f..d9b87606ff7a 100644
--- a/drivers/input/misc/iqs7222.c
+++ b/drivers/input/misc/iqs7222.c
@@ -2385,9 +2385,9 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222,
for (i = 0; i < ARRAY_SIZE(iqs7222_kp_events); i++) {
const char *event_name = iqs7222_kp_events[i].name;
u16 event_enable = iqs7222_kp_events[i].enable;
- struct fwnode_handle *event_node;
- event_node = fwnode_get_named_child_node(chan_node, event_name);
+ struct fwnode_handle *event_node __free(fwnode_handle) =
+ fwnode_get_named_child_node(chan_node, event_name);
if (!event_node)
continue;
@@ -2408,7 +2408,6 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222,
dev_err(&client->dev,
"Invalid %s press timeout: %u\n",
fwnode_get_name(event_node), val);
- fwnode_handle_put(event_node);
return -EINVAL;
}
@@ -2418,7 +2417,6 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222,
dev_err(&client->dev,
"Failed to read %s press timeout: %d\n",
fwnode_get_name(event_node), error);
- fwnode_handle_put(event_node);
return error;
}
@@ -2429,7 +2427,6 @@ static int iqs7222_parse_chan(struct iqs7222_private *iqs7222,
dev_desc->touch_link - (i ? 0 : 2),
&iqs7222->kp_type[chan_index][i],
&iqs7222->kp_code[chan_index][i]);
- fwnode_handle_put(event_node);
if (error)
return error;
@@ -2604,10 +2601,10 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222,
for (i = 0; i < ARRAY_SIZE(iqs7222_sl_events); i++) {
const char *event_name = iqs7222_sl_events[i].name;
- struct fwnode_handle *event_node;
enum iqs7222_reg_key_id reg_key;
- event_node = fwnode_get_named_child_node(sldr_node, event_name);
+ struct fwnode_handle *event_node __free(fwnode_handle) =
+ fwnode_get_named_child_node(sldr_node, event_name);
if (!event_node)
continue;
@@ -2639,7 +2636,6 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222,
: sldr_setup[4 + reg_offset],
NULL,
&iqs7222->sl_code[sldr_index][i]);
- fwnode_handle_put(event_node);
if (error)
return error;
@@ -2742,9 +2738,9 @@ static int iqs7222_parse_tpad(struct iqs7222_private *iqs7222,
for (i = 0; i < ARRAY_SIZE(iqs7222_tp_events); i++) {
const char *event_name = iqs7222_tp_events[i].name;
- struct fwnode_handle *event_node;
- event_node = fwnode_get_named_child_node(tpad_node, event_name);
+ struct fwnode_handle *event_node __free(fwnode_handle) =
+ fwnode_get_named_child_node(tpad_node, event_name);
if (!event_node)
continue;
@@ -2760,7 +2756,6 @@ static int iqs7222_parse_tpad(struct iqs7222_private *iqs7222,
iqs7222_tp_events[i].link, 1566,
NULL,
&iqs7222->tp_code[i]);
- fwnode_handle_put(event_node);
if (error)
return error;
@@ -2818,9 +2813,9 @@ static int iqs7222_parse_reg_grp(struct iqs7222_private *iqs7222,
int reg_grp_index)
{
struct i2c_client *client = iqs7222->client;
- struct fwnode_handle *reg_grp_node;
int error;
+ struct fwnode_handle *reg_grp_node __free(fwnode_handle) = NULL;
if (iqs7222_reg_grp_names[reg_grp]) {
char reg_grp_name[16];
@@ -2838,14 +2833,17 @@ static int iqs7222_parse_reg_grp(struct iqs7222_private *iqs7222,
error = iqs7222_parse_props(iqs7222, reg_grp_node, reg_grp_index,
reg_grp, IQS7222_REG_KEY_NONE);
+ if (error)
+ return error;
- if (!error && iqs7222_parse_extra[reg_grp])
+ if (iqs7222_parse_extra[reg_grp]) {
error = iqs7222_parse_extra[reg_grp](iqs7222, reg_grp_node,
reg_grp_index);
+ if (error)
+ return error;
+ }
- fwnode_handle_put(reg_grp_node);
-
- return error;
+ return 0;
}
static int iqs7222_parse_all(struct iqs7222_private *iqs7222)
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 14/22] Input: iqs626a - use cleanup facility for fwnodes
From: Dmitry Torokhov @ 2024-09-04 4:48 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Use __free(fwnode_handle) cleanup facility to ensure that references to
acquired fwnodes are dropped at appropriate times automatically.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/iqs626a.c | 22 ++++++----------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/input/misc/iqs626a.c b/drivers/input/misc/iqs626a.c
index 0dab54d3a060..7a6e6927f331 100644
--- a/drivers/input/misc/iqs626a.c
+++ b/drivers/input/misc/iqs626a.c
@@ -462,7 +462,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
{
struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
struct i2c_client *client = iqs626->client;
- struct fwnode_handle *ev_node;
const char *ev_name;
u8 *thresh, *hyst;
unsigned int val;
@@ -501,6 +500,7 @@ iqs626_parse_events(struct iqs626_private *iqs626,
if (!iqs626_channels[ch_id].events[i])
continue;
+ struct fwnode_handle *ev_node __free(fwnode_handle) = NULL;
if (ch_id == IQS626_CH_TP_2 || ch_id == IQS626_CH_TP_3) {
/*
* Trackpad touch events are simply described under the
@@ -530,7 +530,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
dev_err(&client->dev,
"Invalid input type: %u\n",
val);
- fwnode_handle_put(ev_node);
return -EINVAL;
}
@@ -545,7 +544,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
dev_err(&client->dev,
"Invalid %s channel hysteresis: %u\n",
fwnode_get_name(ch_node), val);
- fwnode_handle_put(ev_node);
return -EINVAL;
}
@@ -566,7 +564,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
dev_err(&client->dev,
"Invalid %s channel threshold: %u\n",
fwnode_get_name(ch_node), val);
- fwnode_handle_put(ev_node);
return -EINVAL;
}
@@ -575,8 +572,6 @@ iqs626_parse_events(struct iqs626_private *iqs626,
else
*(thresh + iqs626_events[i].th_offs) = val;
}
-
- fwnode_handle_put(ev_node);
}
return 0;
@@ -774,12 +769,12 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
for (i = 0; i < iqs626_channels[ch_id].num_ch; i++) {
u8 *ati_base = &sys_reg->tp_grp_reg.ch_reg_tp[i].ati_base;
u8 *thresh = &sys_reg->tp_grp_reg.ch_reg_tp[i].thresh;
- struct fwnode_handle *tc_node;
char tc_name[10];
snprintf(tc_name, sizeof(tc_name), "channel-%d", i);
- tc_node = fwnode_get_named_child_node(ch_node, tc_name);
+ struct fwnode_handle *tc_node __free(fwnode_handle) =
+ fwnode_get_named_child_node(ch_node, tc_name);
if (!tc_node)
continue;
@@ -790,7 +785,6 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
dev_err(&client->dev,
"Invalid %s %s ATI base: %u\n",
fwnode_get_name(ch_node), tc_name, val);
- fwnode_handle_put(tc_node);
return -EINVAL;
}
@@ -803,14 +797,11 @@ static int iqs626_parse_trackpad(struct iqs626_private *iqs626,
dev_err(&client->dev,
"Invalid %s %s threshold: %u\n",
fwnode_get_name(ch_node), tc_name, val);
- fwnode_handle_put(tc_node);
return -EINVAL;
}
*thresh = val;
}
-
- fwnode_handle_put(tc_node);
}
if (!fwnode_property_present(ch_node, "linux,keycodes"))
@@ -1233,7 +1224,6 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
{
struct iqs626_sys_reg *sys_reg = &iqs626->sys_reg;
struct i2c_client *client = iqs626->client;
- struct fwnode_handle *ch_node;
unsigned int val;
int error, i;
u16 general;
@@ -1375,13 +1365,13 @@ static int iqs626_parse_prop(struct iqs626_private *iqs626)
sys_reg->active = 0;
for (i = 0; i < ARRAY_SIZE(iqs626_channels); i++) {
- ch_node = device_get_named_child_node(&client->dev,
- iqs626_channels[i].name);
+ struct fwnode_handle *ch_node __free(fwnode_handle) =
+ device_get_named_child_node(&client->dev,
+ iqs626_channels[i].name);
if (!ch_node)
continue;
error = iqs626_parse_channel(iqs626, ch_node, i);
- fwnode_handle_put(ch_node);
if (error)
return error;
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 13/22] Input: iqs269a - use cleanup facility for fwnodes
From: Dmitry Torokhov @ 2024-09-04 4:48 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Use __free(fwnode_handle) cleanup facility to ensure that references to
acquired fwnodes are dropped at appropriate times automatically.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/iqs269a.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/input/misc/iqs269a.c b/drivers/input/misc/iqs269a.c
index c34d847fa4af..1851848e2cd3 100644
--- a/drivers/input/misc/iqs269a.c
+++ b/drivers/input/misc/iqs269a.c
@@ -550,7 +550,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
const struct fwnode_handle *ch_node)
{
struct i2c_client *client = iqs269->client;
- struct fwnode_handle *ev_node;
struct iqs269_ch_reg *ch_reg;
u16 engine_a, engine_b;
unsigned int reg, val;
@@ -727,8 +726,9 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
}
for (i = 0; i < ARRAY_SIZE(iqs269_events); i++) {
- ev_node = fwnode_get_named_child_node(ch_node,
- iqs269_events[i].name);
+ struct fwnode_handle *ev_node __free(fwnode_handle) =
+ fwnode_get_named_child_node(ch_node,
+ iqs269_events[i].name);
if (!ev_node)
continue;
@@ -737,7 +737,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
dev_err(&client->dev,
"Invalid channel %u threshold: %u\n",
reg, val);
- fwnode_handle_put(ev_node);
return -EINVAL;
}
@@ -751,7 +750,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
dev_err(&client->dev,
"Invalid channel %u hysteresis: %u\n",
reg, val);
- fwnode_handle_put(ev_node);
return -EINVAL;
}
@@ -767,7 +765,6 @@ static int iqs269_parse_chan(struct iqs269_private *iqs269,
}
error = fwnode_property_read_u32(ev_node, "linux,code", &val);
- fwnode_handle_put(ev_node);
if (error == -EINVAL) {
continue;
} else if (error) {
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 12/22] Input: iqs269a - use guard notation when acquiring mutex
From: Dmitry Torokhov @ 2024-09-04 4:47 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that mutexes are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/iqs269a.c | 46 +++++++++++++-----------------------
1 file changed, 16 insertions(+), 30 deletions(-)
diff --git a/drivers/input/misc/iqs269a.c b/drivers/input/misc/iqs269a.c
index 843f8a3f3410..c34d847fa4af 100644
--- a/drivers/input/misc/iqs269a.c
+++ b/drivers/input/misc/iqs269a.c
@@ -365,7 +365,7 @@ static int iqs269_ati_mode_set(struct iqs269_private *iqs269,
if (mode > IQS269_CHx_ENG_A_ATI_MODE_MAX)
return -EINVAL;
- mutex_lock(&iqs269->lock);
+ guard(mutex)(&iqs269->lock);
engine_a = be16_to_cpu(ch_reg[ch_num].engine_a);
@@ -375,8 +375,6 @@ static int iqs269_ati_mode_set(struct iqs269_private *iqs269,
ch_reg[ch_num].engine_a = cpu_to_be16(engine_a);
iqs269->ati_current = false;
- mutex_unlock(&iqs269->lock);
-
return 0;
}
@@ -389,9 +387,9 @@ static int iqs269_ati_mode_get(struct iqs269_private *iqs269,
if (ch_num >= IQS269_NUM_CH)
return -EINVAL;
- mutex_lock(&iqs269->lock);
+ guard(mutex)(&iqs269->lock);
+
engine_a = be16_to_cpu(ch_reg[ch_num].engine_a);
- mutex_unlock(&iqs269->lock);
engine_a &= IQS269_CHx_ENG_A_ATI_MODE_MASK;
*mode = (engine_a >> IQS269_CHx_ENG_A_ATI_MODE_SHIFT);
@@ -429,7 +427,7 @@ static int iqs269_ati_base_set(struct iqs269_private *iqs269,
return -EINVAL;
}
- mutex_lock(&iqs269->lock);
+ guard(mutex)(&iqs269->lock);
engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
@@ -439,8 +437,6 @@ static int iqs269_ati_base_set(struct iqs269_private *iqs269,
ch_reg[ch_num].engine_b = cpu_to_be16(engine_b);
iqs269->ati_current = false;
- mutex_unlock(&iqs269->lock);
-
return 0;
}
@@ -453,9 +449,9 @@ static int iqs269_ati_base_get(struct iqs269_private *iqs269,
if (ch_num >= IQS269_NUM_CH)
return -EINVAL;
- mutex_lock(&iqs269->lock);
+ guard(mutex)(&iqs269->lock);
+
engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
- mutex_unlock(&iqs269->lock);
switch (engine_b & IQS269_CHx_ENG_B_ATI_BASE_MASK) {
case IQS269_CHx_ENG_B_ATI_BASE_75:
@@ -491,7 +487,7 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
if (target > IQS269_CHx_ENG_B_ATI_TARGET_MAX)
return -EINVAL;
- mutex_lock(&iqs269->lock);
+ guard(mutex)(&iqs269->lock);
engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
@@ -501,8 +497,6 @@ static int iqs269_ati_target_set(struct iqs269_private *iqs269,
ch_reg[ch_num].engine_b = cpu_to_be16(engine_b);
iqs269->ati_current = false;
- mutex_unlock(&iqs269->lock);
-
return 0;
}
@@ -515,10 +509,9 @@ static int iqs269_ati_target_get(struct iqs269_private *iqs269,
if (ch_num >= IQS269_NUM_CH)
return -EINVAL;
- mutex_lock(&iqs269->lock);
- engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
- mutex_unlock(&iqs269->lock);
+ guard(mutex)(&iqs269->lock);
+ engine_b = be16_to_cpu(ch_reg[ch_num].engine_b);
*target = (engine_b & IQS269_CHx_ENG_B_ATI_TARGET_MASK) * 32;
return 0;
@@ -1199,7 +1192,7 @@ static int iqs269_dev_init(struct iqs269_private *iqs269)
{
int error;
- mutex_lock(&iqs269->lock);
+ guard(mutex)(&iqs269->lock);
/*
* Early revisions of silicon require the following workaround in order
@@ -1210,19 +1203,19 @@ static int iqs269_dev_init(struct iqs269_private *iqs269)
error = regmap_multi_reg_write(iqs269->regmap, iqs269_tws_init,
ARRAY_SIZE(iqs269_tws_init));
if (error)
- goto err_mutex;
+ return error;
}
error = regmap_update_bits(iqs269->regmap, IQS269_HALL_UI,
IQS269_HALL_UI_ENABLE,
iqs269->hall_enable ? ~0 : 0);
if (error)
- goto err_mutex;
+ return error;
error = regmap_raw_write(iqs269->regmap, IQS269_SYS_SETTINGS,
&iqs269->sys_reg, sizeof(iqs269->sys_reg));
if (error)
- goto err_mutex;
+ return error;
/*
* The following delay gives the device time to deassert its RDY output
@@ -1232,10 +1225,7 @@ static int iqs269_dev_init(struct iqs269_private *iqs269)
iqs269->ati_current = true;
-err_mutex:
- mutex_unlock(&iqs269->lock);
-
- return error;
+ return 0;
}
static int iqs269_input_init(struct iqs269_private *iqs269)
@@ -1580,13 +1570,11 @@ static ssize_t hall_enable_store(struct device *dev,
if (error)
return error;
- mutex_lock(&iqs269->lock);
+ guard(mutex)(&iqs269->lock);
iqs269->hall_enable = val;
iqs269->ati_current = false;
- mutex_unlock(&iqs269->lock);
-
return count;
}
@@ -1643,13 +1631,11 @@ static ssize_t rx_enable_store(struct device *dev,
if (val > 0xFF)
return -EINVAL;
- mutex_lock(&iqs269->lock);
+ guard(mutex)(&iqs269->lock);
ch_reg[iqs269->ch_num].rx_enable = val;
iqs269->ati_current = false;
- mutex_unlock(&iqs269->lock);
-
return count;
}
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* [PATCH 11/22] Input: ibm-panel - use guard notation when acquiring spinlock
From: Dmitry Torokhov @ 2024-09-04 4:47 UTC (permalink / raw)
To: linux-input
Cc: Michael Hennerich, Ville Syrjala, Support Opensource, Eddie James,
Andrey Moiseev, Hans de Goede, Javier Carrasco, Jeff LaBundy,
linux-kernel
In-Reply-To: <20240904044244.1042174-1-dmitry.torokhov@gmail.com>
Using guard notation makes the code more compact and error handling
more robust by ensuring that locks are released in all code paths
when control leaves critical section.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/misc/ibm-panel.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/input/misc/ibm-panel.c b/drivers/input/misc/ibm-panel.c
index 867ac7aa10d2..aa48f62d7ea0 100644
--- a/drivers/input/misc/ibm-panel.c
+++ b/drivers/input/misc/ibm-panel.c
@@ -77,12 +77,11 @@ static void ibm_panel_process_command(struct ibm_panel *panel)
static int ibm_panel_i2c_slave_cb(struct i2c_client *client,
enum i2c_slave_event event, u8 *val)
{
- unsigned long flags;
struct ibm_panel *panel = i2c_get_clientdata(client);
dev_dbg(&panel->input->dev, "event: %u data: %02x\n", event, *val);
- spin_lock_irqsave(&panel->lock, flags);
+ guard(spinlock_irqsave)(&panel->lock);
switch (event) {
case I2C_SLAVE_STOP:
@@ -114,8 +113,6 @@ static int ibm_panel_i2c_slave_cb(struct i2c_client *client,
break;
}
- spin_unlock_irqrestore(&panel->lock, flags);
-
return 0;
}
--
2.46.0.469.g59c65b2a67-goog
^ permalink raw reply related
* Re: [PATCH 0/6] Convert joystick drivers to use new cleanup facilities
From: Dmitry Torokhov @ 2024-09-04 4:43 UTC (permalink / raw)
To: linux-input; +Cc: Erick Archer, Christophe JAILLET, linux-kernel
In-Reply-To: <20240904043104.1030257-1-dmitry.torokhov@gmail.com>
On Tue, Sep 03, 2024 at 09:30:57PM -0700, Dmitry Torokhov wrote:
> Hi,
>
> This series converts drivers found in drivers/input/keyboard to use new
This should have read drivers/input/joystick obviously...
> __free() and guard() cleanup facilities that simplify the code and
> ensure that all resources are released appropriately.
>
> Thanks!
>
> Dmitry Torokhov (6):
> Input: db9 - use guard notation when acquiring mutex
> Input: gamecon - use guard notation when acquiring mutex
> Input: iforce - use guard notation when acquiring mutex and spinlock
> Input: n64joy - use guard notation when acquiring mutex
> Input: turbografx - use guard notation when acquiring mutex
> Input: xpad - use guard notation when acquiring mutex and spinlock
>
> drivers/input/joystick/db9.c | 30 +++---
> drivers/input/joystick/gamecon.c | 22 ++---
> drivers/input/joystick/iforce/iforce-ff.c | 48 +++++----
> .../input/joystick/iforce/iforce-packets.c | 57 +++++------
> drivers/input/joystick/iforce/iforce-serio.c | 36 +++----
> drivers/input/joystick/iforce/iforce-usb.c | 13 ++-
> drivers/input/joystick/n64joy.c | 35 +++----
> drivers/input/joystick/turbografx.c | 22 ++---
> drivers/input/joystick/xpad.c | 99 +++++++------------
> 9 files changed, 152 insertions(+), 210 deletions(-)
>
> --
> Dmitry
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox