[PATCH] hid: core: add managed version of hid_hw_start

[PATCH] hid: core: add managed version of hid_hw_start

[Heiner Kallweit]

Most calls to hid_hw_stop are boilerplate code. The function comment
states: "This is usually called from remove function or from probe
when something failed and hid_hw_start was called already."

A managed version of hid_hw_start allows to avoid this boilerplate
code in most cases (similar to the other devm_ functions). 
hid_hw_stop is called automatically before the device is deleted.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/hid/hid-core.c | 22 ++++++++++++++++++++++
 include/linux/hid.h    |  3 +++
 2 files changed, 25 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index e95f23f..61c6f63 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2752,6 +2752,28 @@ int hid_check_keys_pressed(struct hid_device *hid)
 
 EXPORT_SYMBOL_GPL(hid_check_keys_pressed);
 
+static void __hid_hw_stop(void *data)
+{
+	hid_hw_stop((struct hid_device *)data);
+}
+
+int __must_check devm_hid_hw_start(struct hid_device *hdev,
+				   unsigned int connect_mask)
+{
+	int ret;
+
+	ret = hid_hw_start(hdev, connect_mask);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action(&hdev->dev, __hid_hw_stop, hdev);
+	if (ret)
+		hid_hw_stop(hdev);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(devm_hid_hw_start);
+
 static int __init hid_init(void)
 {
 	int ret;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 75b66ec..2d85b3e 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -774,6 +774,9 @@ extern bool hid_ignore(struct hid_device *);
 extern int hid_add_device(struct hid_device *);
 extern void hid_destroy_device(struct hid_device *);
 
+extern int __must_check devm_hid_hw_start(struct hid_device *hdev,
+					  unsigned int connect_mask);
+
 extern int __must_check __hid_register_driver(struct hid_driver *,
 		struct module *, const char *mod_name);
 
-- 
2.6.4

Re: [PATCH] hid: core: add managed version of hid_hw_start

[Jiri Kosina]

On Wed, 6 Jan 2016, Heiner Kallweit wrote:

> Most calls to hid_hw_stop are boilerplate code. The function comment
> states: "This is usually called from remove function or from probe
> when something failed and hid_hw_start was called already."
> 
> A managed version of hid_hw_start allows to avoid this boilerplate
> code in most cases (similar to the other devm_ functions). 
> hid_hw_stop is called automatically before the device is deleted.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Generally I agree with the idea, but I'll be merging this only together 
with patches that are converting drivers to actually use this API.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] hid: core: add managed version of hid_hw_start

[Heiner Kallweit]

Am 08.01.2016 um 10:51 schrieb Jiri Kosina:
> On Wed, 6 Jan 2016, Heiner Kallweit wrote:
> 
>> Most calls to hid_hw_stop are boilerplate code. The function comment
>> states: "This is usually called from remove function or from probe
>> when something failed and hid_hw_start was called already."
>>
>> A managed version of hid_hw_start allows to avoid this boilerplate
>> code in most cases (similar to the other devm_ functions). 
>> hid_hw_stop is called automatically before the device is deleted.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> 
> Generally I agree with the idea, but I'll be merging this only together 
> with patches that are converting drivers to actually use this API.
> 
OK. When working on it I saw that hid_hw_stop is called by the core if
the driver doesn't define a remove callback.
This means if we use devm_hid_hw_start in a driver with a probe but no
remove callback then hid_hw_stop will be called twice.
To prevent this I'd switch all drivers with probe but no remove to the
new call and remove the fallback call to hid_hw_stop in the core.
At the same time I'd switch the fallback call to hid_hw_start in the core
(in case the driver doesn't define a probe callback) to the new call.
This way the change is transparent to drivers which define neither probe
nor remove.
Fine with you?

> Thanks,
> 
Regards, Heiner