* Re: [PATCH] Input: egalax_ts - Provide a .remove function
From: Dmitry Torokhov @ 2013-07-24 21:08 UTC (permalink / raw)
To: Fabio Estevam; +Cc: shawn.guo, linux-input
In-Reply-To: <1374698769-25664-1-git-send-email-fabio.estevam@freescale.com>
Hi Fabio,
On Wednesday, July 24, 2013 05:46:09 PM Fabio Estevam wrote:
> Provide a .remove function so that we can unregister the input device.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
> drivers/input/touchscreen/egalax_ts.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/input/touchscreen/egalax_ts.c
> b/drivers/input/touchscreen/egalax_ts.c index ef5fcb0..e46be61 100644
> --- a/drivers/input/touchscreen/egalax_ts.c
> +++ b/drivers/input/touchscreen/egalax_ts.c
> @@ -234,6 +234,16 @@ static int egalax_ts_probe(struct i2c_client *client,
> return 0;
> }
>
> +static int egalax_ts_remove(struct i2c_client *client)
> +{
> + struct egalax_ts *ts = i2c_get_clientdata(client);
> +
> + input_unregister_device(ts->input_dev);
> + kfree(ts);
The egalax_ts driver has been converted to devm* infrastructure, using kfree()
to free memory not only unnecessary, but wrong.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] Input: egalax_ts - Provide a .remove function
From: Fabio Estevam @ 2013-07-24 20:46 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: shawn.guo, linux-input, Fabio Estevam
Provide a .remove function so that we can unregister the input device.
Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
drivers/input/touchscreen/egalax_ts.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/input/touchscreen/egalax_ts.c b/drivers/input/touchscreen/egalax_ts.c
index ef5fcb0..e46be61 100644
--- a/drivers/input/touchscreen/egalax_ts.c
+++ b/drivers/input/touchscreen/egalax_ts.c
@@ -234,6 +234,16 @@ static int egalax_ts_probe(struct i2c_client *client,
return 0;
}
+static int egalax_ts_remove(struct i2c_client *client)
+{
+ struct egalax_ts *ts = i2c_get_clientdata(client);
+
+ input_unregister_device(ts->input_dev);
+ kfree(ts);
+
+ return 0;
+}
+
static const struct i2c_device_id egalax_ts_id[] = {
{ "egalax_ts", 0 },
{ }
@@ -277,6 +287,7 @@ static struct i2c_driver egalax_ts_driver = {
},
.id_table = egalax_ts_id,
.probe = egalax_ts_probe,
+ .remove = egalax_ts_remove,
};
module_i2c_driver(egalax_ts_driver);
--
1.8.1.2
^ permalink raw reply related
* Re: [PATCH v2 1/2] HID: trivial devm conversion for special hid drivers
From: Andy Shevchenko @ 2013-07-24 18:13 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Stephane Chatty,
linux-input, linux-kernel@vger.kernel.org
In-Reply-To: <1374687485-31440-2-git-send-email-benjamin.tissoires@redhat.com>
On Wed, Jul 24, 2013 at 8:38 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> It is safe to use devres allocation within the hid subsystem:
> - the devres release is called _after_ the call to .remove(), meaning
> that no freed pointers will exists while removing the device
> - if a .probe() fails, devres releases all the allocated ressources
> before going to the next driver: there will not be ghost ressources
> attached to a hid device if several drivers are probed.
>
> Given that, we can clean up a little some of the HID drivers. These ones
> are trivial:
> - there is only one kzalloc in the driver
> - the .remove() callback contains only one kfree on top of hid_hw_stop()
> - the error path in the probe is easy enough to be manually checked
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/hid-a4tech.c | 21 ++++-----------------
> drivers/hid/hid-apple.c | 16 +++-------------
> drivers/hid/hid-magicmouse.c | 17 +++--------------
> drivers/hid/hid-sony.c | 9 +++------
> drivers/hid/hid-zydacron.c | 19 +++----------------
> 5 files changed, 16 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
> index 7c5507e..9428ea7 100644
> --- a/drivers/hid/hid-a4tech.c
> +++ b/drivers/hid/hid-a4tech.c
> @@ -90,11 +90,10 @@ static int a4_probe(struct hid_device *hdev, const struct hid_device_id *id)
> struct a4tech_sc *a4;
> int ret;
>
> - a4 = kzalloc(sizeof(*a4), GFP_KERNEL);
> + a4 = devm_kzalloc(&hdev->dev, sizeof(*a4), GFP_KERNEL);
> if (a4 == NULL) {
> hid_err(hdev, "can't alloc device descriptor\n");
> - ret = -ENOMEM;
> - goto err_free;
> + return -ENOMEM;
> }
>
> a4->quirks = id->driver_data;
> @@ -104,27 +103,16 @@ static int a4_probe(struct hid_device *hdev, const struct hid_device_id *id)
> ret = hid_parse(hdev);
> if (ret) {
> hid_err(hdev, "parse failed\n");
> - goto err_free;
> + return ret;
> }
>
> ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> if (ret) {
> hid_err(hdev, "hw start failed\n");
> - goto err_free;
> + return ret;
> }
>
> return 0;
> -err_free:
> - kfree(a4);
> - return ret;
> -}
> -
> -static void a4_remove(struct hid_device *hdev)
> -{
> - struct a4tech_sc *a4 = hid_get_drvdata(hdev);
> -
> - hid_hw_stop(hdev);
> - kfree(a4);
> }
>
> static const struct hid_device_id a4_devices[] = {
> @@ -144,7 +132,6 @@ static struct hid_driver a4_driver = {
> .input_mapped = a4_input_mapped,
> .event = a4_event,
> .probe = a4_probe,
> - .remove = a4_remove,
> };
> module_hid_driver(a4_driver);
>
> diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> index c7710b5..881cf7b 100644
> --- a/drivers/hid/hid-apple.c
> +++ b/drivers/hid/hid-apple.c
> @@ -349,7 +349,7 @@ static int apple_probe(struct hid_device *hdev,
> unsigned int connect_mask = HID_CONNECT_DEFAULT;
> int ret;
>
> - asc = kzalloc(sizeof(*asc), GFP_KERNEL);
> + asc = devm_kzalloc(&hdev->dev, sizeof(*asc), GFP_KERNEL);
> if (asc == NULL) {
> hid_err(hdev, "can't alloc apple descriptor\n");
> return -ENOMEM;
> @@ -362,7 +362,7 @@ static int apple_probe(struct hid_device *hdev,
> ret = hid_parse(hdev);
> if (ret) {
> hid_err(hdev, "parse failed\n");
> - goto err_free;
> + return ret;
> }
>
> if (quirks & APPLE_HIDDEV)
> @@ -373,19 +373,10 @@ static int apple_probe(struct hid_device *hdev,
> ret = hid_hw_start(hdev, connect_mask);
> if (ret) {
> hid_err(hdev, "hw start failed\n");
> - goto err_free;
> + return ret;
> }
>
> return 0;
> -err_free:
> - kfree(asc);
> - return ret;
> -}
> -
> -static void apple_remove(struct hid_device *hdev)
> -{
> - hid_hw_stop(hdev);
> - kfree(hid_get_drvdata(hdev));
> }
>
> static const struct hid_device_id apple_devices[] = {
> @@ -551,7 +542,6 @@ static struct hid_driver apple_driver = {
> .id_table = apple_devices,
> .report_fixup = apple_report_fixup,
> .probe = apple_probe,
> - .remove = apple_remove,
> .event = apple_event,
> .input_mapping = apple_input_mapping,
> .input_mapped = apple_input_mapped,
> diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
> index 5bc3734..d393eb7 100644
> --- a/drivers/hid/hid-magicmouse.c
> +++ b/drivers/hid/hid-magicmouse.c
> @@ -484,7 +484,7 @@ static int magicmouse_probe(struct hid_device *hdev,
> struct hid_report *report;
> int ret;
>
> - msc = kzalloc(sizeof(*msc), GFP_KERNEL);
> + msc = devm_kzalloc(&hdev->dev, sizeof(*msc), GFP_KERNEL);
> if (msc == NULL) {
> hid_err(hdev, "can't alloc magicmouse descriptor\n");
> return -ENOMEM;
> @@ -498,13 +498,13 @@ static int magicmouse_probe(struct hid_device *hdev,
> ret = hid_parse(hdev);
> if (ret) {
> hid_err(hdev, "magicmouse hid parse failed\n");
> - goto err_free;
> + return ret;
> }
>
> ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> if (ret) {
> hid_err(hdev, "magicmouse hw start failed\n");
> - goto err_free;
> + return ret;
> }
>
> if (!msc->input) {
> @@ -548,19 +548,9 @@ static int magicmouse_probe(struct hid_device *hdev,
> return 0;
> err_stop_hw:
> hid_hw_stop(hdev);
> -err_free:
> - kfree(msc);
> return ret;
> }
>
> -static void magicmouse_remove(struct hid_device *hdev)
> -{
> - struct magicmouse_sc *msc = hid_get_drvdata(hdev);
> -
> - hid_hw_stop(hdev);
> - kfree(msc);
> -}
> -
> static const struct hid_device_id magic_mice[] = {
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
> USB_DEVICE_ID_APPLE_MAGICMOUSE), .driver_data = 0 },
> @@ -574,7 +564,6 @@ static struct hid_driver magicmouse_driver = {
> .name = "magicmouse",
> .id_table = magic_mice,
> .probe = magicmouse_probe,
> - .remove = magicmouse_remove,
> .raw_event = magicmouse_raw_event,
> .input_mapping = magicmouse_input_mapping,
> .input_configured = magicmouse_input_configured,
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index 87fbe29..30dbb6b 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -624,7 +624,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
> struct sony_sc *sc;
> unsigned int connect_mask = HID_CONNECT_DEFAULT;
>
> - sc = kzalloc(sizeof(*sc), GFP_KERNEL);
> + sc = devm_kzalloc(&hdev->dev, sizeof(*sc), GFP_KERNEL);
> if (sc == NULL) {
> hid_err(hdev, "can't alloc sony descriptor\n");
> return -ENOMEM;
> @@ -636,7 +636,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
> ret = hid_parse(hdev);
> if (ret) {
> hid_err(hdev, "parse failed\n");
> - goto err_free;
> + return ret;
> }
>
> if (sc->quirks & VAIO_RDESC_CONSTANT)
> @@ -649,7 +649,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
> ret = hid_hw_start(hdev, connect_mask);
> if (ret) {
> hid_err(hdev, "hw start failed\n");
> - goto err_free;
> + return ret;
> }
>
> if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
> @@ -669,8 +669,6 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
> return 0;
> err_stop:
> hid_hw_stop(hdev);
> -err_free:
> - kfree(sc);
> return ret;
> }
>
> @@ -682,7 +680,6 @@ static void sony_remove(struct hid_device *hdev)
> buzz_remove(hdev);
>
> hid_hw_stop(hdev);
> - kfree(sc);
> }
>
> static const struct hid_device_id sony_devices[] = {
> diff --git a/drivers/hid/hid-zydacron.c b/drivers/hid/hid-zydacron.c
> index e4cddec..1a660bd 100644
> --- a/drivers/hid/hid-zydacron.c
> +++ b/drivers/hid/hid-zydacron.c
> @@ -169,7 +169,7 @@ static int zc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> int ret;
> struct zc_device *zc;
>
> - zc = kzalloc(sizeof(*zc), GFP_KERNEL);
> + zc = devm_kzalloc(&hdev->dev, sizeof(*zc), GFP_KERNEL);
> if (zc == NULL) {
> hid_err(hdev, "can't alloc descriptor\n");
> return -ENOMEM;
> @@ -180,28 +180,16 @@ static int zc_probe(struct hid_device *hdev, const struct hid_device_id *id)
> ret = hid_parse(hdev);
> if (ret) {
> hid_err(hdev, "parse failed\n");
> - goto err_free;
> + return ret;
> }
>
> ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> if (ret) {
> hid_err(hdev, "hw start failed\n");
> - goto err_free;
> + return ret;
> }
>
> return 0;
> -err_free:
> - kfree(zc);
> -
> - return ret;
> -}
> -
> -static void zc_remove(struct hid_device *hdev)
> -{
> - struct zc_device *zc = hid_get_drvdata(hdev);
> -
> - hid_hw_stop(hdev);
> - kfree(zc);
> }
>
> static const struct hid_device_id zc_devices[] = {
> @@ -217,7 +205,6 @@ static struct hid_driver zc_driver = {
> .input_mapping = zc_input_mapping,
> .raw_event = zc_raw_event,
> .probe = zc_probe,
> - .remove = zc_remove,
> };
> module_hid_driver(zc_driver);
>
> --
> 1.8.3.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v2 1/2] HID: trivial devm conversion for special hid drivers
From: Benjamin Tissoires @ 2013-07-24 17:38 UTC (permalink / raw)
To: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Andy Shevchenko,
Stephane Chatty, linux-input, linux-kernel
In-Reply-To: <1374687485-31440-1-git-send-email-benjamin.tissoires@redhat.com>
It is safe to use devres allocation within the hid subsystem:
- the devres release is called _after_ the call to .remove(), meaning
that no freed pointers will exists while removing the device
- if a .probe() fails, devres releases all the allocated ressources
before going to the next driver: there will not be ghost ressources
attached to a hid device if several drivers are probed.
Given that, we can clean up a little some of the HID drivers. These ones
are trivial:
- there is only one kzalloc in the driver
- the .remove() callback contains only one kfree on top of hid_hw_stop()
- the error path in the probe is easy enough to be manually checked
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-a4tech.c | 21 ++++-----------------
drivers/hid/hid-apple.c | 16 +++-------------
drivers/hid/hid-magicmouse.c | 17 +++--------------
drivers/hid/hid-sony.c | 9 +++------
drivers/hid/hid-zydacron.c | 19 +++----------------
5 files changed, 16 insertions(+), 66 deletions(-)
diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
index 7c5507e..9428ea7 100644
--- a/drivers/hid/hid-a4tech.c
+++ b/drivers/hid/hid-a4tech.c
@@ -90,11 +90,10 @@ static int a4_probe(struct hid_device *hdev, const struct hid_device_id *id)
struct a4tech_sc *a4;
int ret;
- a4 = kzalloc(sizeof(*a4), GFP_KERNEL);
+ a4 = devm_kzalloc(&hdev->dev, sizeof(*a4), GFP_KERNEL);
if (a4 == NULL) {
hid_err(hdev, "can't alloc device descriptor\n");
- ret = -ENOMEM;
- goto err_free;
+ return -ENOMEM;
}
a4->quirks = id->driver_data;
@@ -104,27 +103,16 @@ static int a4_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
- goto err_free;
+ return ret;
}
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "hw start failed\n");
- goto err_free;
+ return ret;
}
return 0;
-err_free:
- kfree(a4);
- return ret;
-}
-
-static void a4_remove(struct hid_device *hdev)
-{
- struct a4tech_sc *a4 = hid_get_drvdata(hdev);
-
- hid_hw_stop(hdev);
- kfree(a4);
}
static const struct hid_device_id a4_devices[] = {
@@ -144,7 +132,6 @@ static struct hid_driver a4_driver = {
.input_mapped = a4_input_mapped,
.event = a4_event,
.probe = a4_probe,
- .remove = a4_remove,
};
module_hid_driver(a4_driver);
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index c7710b5..881cf7b 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -349,7 +349,7 @@ static int apple_probe(struct hid_device *hdev,
unsigned int connect_mask = HID_CONNECT_DEFAULT;
int ret;
- asc = kzalloc(sizeof(*asc), GFP_KERNEL);
+ asc = devm_kzalloc(&hdev->dev, sizeof(*asc), GFP_KERNEL);
if (asc == NULL) {
hid_err(hdev, "can't alloc apple descriptor\n");
return -ENOMEM;
@@ -362,7 +362,7 @@ static int apple_probe(struct hid_device *hdev,
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
- goto err_free;
+ return ret;
}
if (quirks & APPLE_HIDDEV)
@@ -373,19 +373,10 @@ static int apple_probe(struct hid_device *hdev,
ret = hid_hw_start(hdev, connect_mask);
if (ret) {
hid_err(hdev, "hw start failed\n");
- goto err_free;
+ return ret;
}
return 0;
-err_free:
- kfree(asc);
- return ret;
-}
-
-static void apple_remove(struct hid_device *hdev)
-{
- hid_hw_stop(hdev);
- kfree(hid_get_drvdata(hdev));
}
static const struct hid_device_id apple_devices[] = {
@@ -551,7 +542,6 @@ static struct hid_driver apple_driver = {
.id_table = apple_devices,
.report_fixup = apple_report_fixup,
.probe = apple_probe,
- .remove = apple_remove,
.event = apple_event,
.input_mapping = apple_input_mapping,
.input_mapped = apple_input_mapped,
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 5bc3734..d393eb7 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -484,7 +484,7 @@ static int magicmouse_probe(struct hid_device *hdev,
struct hid_report *report;
int ret;
- msc = kzalloc(sizeof(*msc), GFP_KERNEL);
+ msc = devm_kzalloc(&hdev->dev, sizeof(*msc), GFP_KERNEL);
if (msc == NULL) {
hid_err(hdev, "can't alloc magicmouse descriptor\n");
return -ENOMEM;
@@ -498,13 +498,13 @@ static int magicmouse_probe(struct hid_device *hdev,
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "magicmouse hid parse failed\n");
- goto err_free;
+ return ret;
}
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "magicmouse hw start failed\n");
- goto err_free;
+ return ret;
}
if (!msc->input) {
@@ -548,19 +548,9 @@ static int magicmouse_probe(struct hid_device *hdev,
return 0;
err_stop_hw:
hid_hw_stop(hdev);
-err_free:
- kfree(msc);
return ret;
}
-static void magicmouse_remove(struct hid_device *hdev)
-{
- struct magicmouse_sc *msc = hid_get_drvdata(hdev);
-
- hid_hw_stop(hdev);
- kfree(msc);
-}
-
static const struct hid_device_id magic_mice[] = {
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
USB_DEVICE_ID_APPLE_MAGICMOUSE), .driver_data = 0 },
@@ -574,7 +564,6 @@ static struct hid_driver magicmouse_driver = {
.name = "magicmouse",
.id_table = magic_mice,
.probe = magicmouse_probe,
- .remove = magicmouse_remove,
.raw_event = magicmouse_raw_event,
.input_mapping = magicmouse_input_mapping,
.input_configured = magicmouse_input_configured,
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 87fbe29..30dbb6b 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -624,7 +624,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
struct sony_sc *sc;
unsigned int connect_mask = HID_CONNECT_DEFAULT;
- sc = kzalloc(sizeof(*sc), GFP_KERNEL);
+ sc = devm_kzalloc(&hdev->dev, sizeof(*sc), GFP_KERNEL);
if (sc == NULL) {
hid_err(hdev, "can't alloc sony descriptor\n");
return -ENOMEM;
@@ -636,7 +636,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
- goto err_free;
+ return ret;
}
if (sc->quirks & VAIO_RDESC_CONSTANT)
@@ -649,7 +649,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_hw_start(hdev, connect_mask);
if (ret) {
hid_err(hdev, "hw start failed\n");
- goto err_free;
+ return ret;
}
if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
@@ -669,8 +669,6 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
return 0;
err_stop:
hid_hw_stop(hdev);
-err_free:
- kfree(sc);
return ret;
}
@@ -682,7 +680,6 @@ static void sony_remove(struct hid_device *hdev)
buzz_remove(hdev);
hid_hw_stop(hdev);
- kfree(sc);
}
static const struct hid_device_id sony_devices[] = {
diff --git a/drivers/hid/hid-zydacron.c b/drivers/hid/hid-zydacron.c
index e4cddec..1a660bd 100644
--- a/drivers/hid/hid-zydacron.c
+++ b/drivers/hid/hid-zydacron.c
@@ -169,7 +169,7 @@ static int zc_probe(struct hid_device *hdev, const struct hid_device_id *id)
int ret;
struct zc_device *zc;
- zc = kzalloc(sizeof(*zc), GFP_KERNEL);
+ zc = devm_kzalloc(&hdev->dev, sizeof(*zc), GFP_KERNEL);
if (zc == NULL) {
hid_err(hdev, "can't alloc descriptor\n");
return -ENOMEM;
@@ -180,28 +180,16 @@ static int zc_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
- goto err_free;
+ return ret;
}
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "hw start failed\n");
- goto err_free;
+ return ret;
}
return 0;
-err_free:
- kfree(zc);
-
- return ret;
-}
-
-static void zc_remove(struct hid_device *hdev)
-{
- struct zc_device *zc = hid_get_drvdata(hdev);
-
- hid_hw_stop(hdev);
- kfree(zc);
}
static const struct hid_device_id zc_devices[] = {
@@ -217,7 +205,6 @@ static struct hid_driver zc_driver = {
.input_mapping = zc_input_mapping,
.raw_event = zc_raw_event,
.probe = zc_probe,
- .remove = zc_remove,
};
module_hid_driver(zc_driver);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 2/2] HID: multitouch: devm conversion
From: Benjamin Tissoires @ 2013-07-24 17:38 UTC (permalink / raw)
To: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Andy Shevchenko,
Stephane Chatty, linux-input, linux-kernel
In-Reply-To: <1374687485-31440-1-git-send-email-benjamin.tissoires@redhat.com>
HID special drivers can use safely the devres API.
Use it to remove 25 lines of code and to clean up a little the error paths.
Besides the basic kzalloc -> devm_kzalloc conversions, I changed the
place of the allocation of the new name. Doing this right in
mt_input_configured() removes the kstrdup call which was not very helpful
and the new way is simpler to understand (and to debug).
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-multitouch.c | 71 ++++++++++++++------------------------------
1 file changed, 23 insertions(+), 48 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index cb0e361..0fe00e2 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -261,17 +261,6 @@ static struct mt_class mt_classes[] = {
{ }
};
-static void mt_free_input_name(struct hid_input *hi)
-{
- struct hid_device *hdev = hi->report->device;
- const char *name = hi->input->name;
-
- if (name != hdev->name) {
- hi->input->name = hdev->name;
- kfree(name);
- }
-}
-
static ssize_t mt_show_quirks(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -415,13 +404,6 @@ static void mt_pen_report(struct hid_device *hid, struct hid_report *report)
static void mt_pen_input_configured(struct hid_device *hdev,
struct hid_input *hi)
{
- char *name = kzalloc(strlen(hi->input->name) + 5, GFP_KERNEL);
- if (name) {
- sprintf(name, "%s Pen", hi->input->name);
- mt_free_input_name(hi);
- hi->input->name = name;
- }
-
/* force BTN_STYLUS to allow tablet matching in udev */
__set_bit(BTN_STYLUS, hi->input->keybit);
}
@@ -928,16 +910,26 @@ static void mt_post_parse(struct mt_device *td)
static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
{
struct mt_device *td = hid_get_drvdata(hdev);
- char *name = kstrdup(hdev->name, GFP_KERNEL);
-
- if (name)
- hi->input->name = name;
+ char *name;
+ const char *suffix = NULL;
if (hi->report->id == td->mt_report_id)
mt_touch_input_configured(hdev, hi);
- if (hi->report->id == td->pen_report_id)
+ if (hi->report->field[0]->physical == HID_DG_STYLUS) {
+ suffix = "Pen";
mt_pen_input_configured(hdev, hi);
+ }
+
+ if (suffix) {
+ name = devm_kzalloc(&hi->input->dev,
+ strlen(hdev->name) + strlen(suffix) + 2,
+ GFP_KERNEL);
+ if (name) {
+ sprintf(name, "%s %s", hdev->name, suffix);
+ hi->input->name = name;
+ }
+ }
}
static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
@@ -945,7 +937,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
int ret, i;
struct mt_device *td;
struct mt_class *mtclass = mt_classes; /* MT_CLS_DEFAULT */
- struct hid_input *hi;
for (i = 0; mt_classes[i].name ; i++) {
if (id->driver_data == mt_classes[i].name) {
@@ -967,7 +958,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
hdev->quirks |= HID_QUIRK_MULTI_INPUT;
hdev->quirks |= HID_QUIRK_NO_EMPTY_INPUT;
- td = kzalloc(sizeof(struct mt_device), GFP_KERNEL);
+ td = devm_kzalloc(&hdev->dev, sizeof(struct mt_device), GFP_KERNEL);
if (!td) {
dev_err(&hdev->dev, "cannot allocate multitouch data\n");
return -ENOMEM;
@@ -980,11 +971,11 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
td->pen_report_id = -1;
hid_set_drvdata(hdev, td);
- td->fields = kzalloc(sizeof(struct mt_fields), GFP_KERNEL);
+ td->fields = devm_kzalloc(&hdev->dev, sizeof(struct mt_fields),
+ GFP_KERNEL);
if (!td->fields) {
dev_err(&hdev->dev, "cannot allocate multitouch fields data\n");
- ret = -ENOMEM;
- goto fail;
+ return -ENOMEM;
}
if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
@@ -992,29 +983,22 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_parse(hdev);
if (ret != 0)
- goto fail;
+ return ret;
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret)
- goto hid_fail;
+ return ret;
ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
mt_set_maxcontacts(hdev);
mt_set_input_mode(hdev);
- kfree(td->fields);
+ /* release .fields memory as it is not used anymore */
+ devm_kfree(&hdev->dev, td->fields);
td->fields = NULL;
return 0;
-
-hid_fail:
- list_for_each_entry(hi, &hdev->inputs, list)
- mt_free_input_name(hi);
-fail:
- kfree(td->fields);
- kfree(td);
- return ret;
}
#ifdef CONFIG_PM
@@ -1039,17 +1023,8 @@ static int mt_resume(struct hid_device *hdev)
static void mt_remove(struct hid_device *hdev)
{
- struct mt_device *td = hid_get_drvdata(hdev);
- struct hid_input *hi;
-
sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
- list_for_each_entry(hi, &hdev->inputs, list)
- mt_free_input_name(hi);
-
hid_hw_stop(hdev);
-
- kfree(td);
- hid_set_drvdata(hdev, NULL);
}
static const struct hid_device_id mt_devices[] = {
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 0/2] HID special drivers converted to devres API
From: Benjamin Tissoires @ 2013-07-24 17:38 UTC (permalink / raw)
To: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Andy Shevchenko,
Stephane Chatty, linux-input, linux-kernel
Hi guys,
this is the v2 of this series.
Cheers,
Benjamin
changes since v1:
- make the exit code of probe() in hid-a4tech.c returning 0
- fix a typo in the commit message of 2/2
Benjamin Tissoires (2):
HID: trivial devm conversion for special hid drivers
HID: multitouch: devm conversion
drivers/hid/hid-a4tech.c | 21 +++----------
drivers/hid/hid-apple.c | 16 ++--------
drivers/hid/hid-magicmouse.c | 17 ++---------
drivers/hid/hid-multitouch.c | 71 ++++++++++++++------------------------------
drivers/hid/hid-sony.c | 9 ++----
drivers/hid/hid-zydacron.c | 19 ++----------
6 files changed, 39 insertions(+), 114 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH 2/2] HID: multitouch: devm conversion
From: Benjamin Tissoires @ 2013-07-24 16:43 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Stephane Chatty,
linux-input, linux-kernel@vger.kernel.org
In-Reply-To: <CAHp75VeuLi=_jN7xfjNvJgpxGgHRw+tg=qV+kkEbPtR72hPj5g@mail.gmail.com>
On Wed, Jul 24, 2013 at 5:44 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Jul 24, 2013 at 6:29 PM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>> HID special drivers can use safely the devres API.
>> Use it to remove 25 lines of code and to clean up a little the error paths.
>>
>> Besides the pur kzalloc -> devm_kzalloc conversions, I changed the
>
> Typo 'pur' -> 'put'
oops, sorry. I'll use "basic" in the next version.
>
>> place of the allocation of the new name. Doing this right in
>> mt_input_configured() removes the kstrdup call which was not very helpful
>> and the new way is simpler to understand (and to debug).
>
> Thanks for the patch! I'm sorry I didn't find time to do this by myself.
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Thanks!
Cheers,
Benjamin
>
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> ---
>> drivers/hid/hid-multitouch.c | 71 ++++++++++++++------------------------------
>> 1 file changed, 23 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index cb0e361..0fe00e2 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -261,17 +261,6 @@ static struct mt_class mt_classes[] = {
>> { }
>> };
>>
>> -static void mt_free_input_name(struct hid_input *hi)
>> -{
>> - struct hid_device *hdev = hi->report->device;
>> - const char *name = hi->input->name;
>> -
>> - if (name != hdev->name) {
>> - hi->input->name = hdev->name;
>> - kfree(name);
>> - }
>> -}
>> -
>> static ssize_t mt_show_quirks(struct device *dev,
>> struct device_attribute *attr,
>> char *buf)
>> @@ -415,13 +404,6 @@ static void mt_pen_report(struct hid_device *hid, struct hid_report *report)
>> static void mt_pen_input_configured(struct hid_device *hdev,
>> struct hid_input *hi)
>> {
>> - char *name = kzalloc(strlen(hi->input->name) + 5, GFP_KERNEL);
>> - if (name) {
>> - sprintf(name, "%s Pen", hi->input->name);
>> - mt_free_input_name(hi);
>> - hi->input->name = name;
>> - }
>> -
>> /* force BTN_STYLUS to allow tablet matching in udev */
>> __set_bit(BTN_STYLUS, hi->input->keybit);
>> }
>> @@ -928,16 +910,26 @@ static void mt_post_parse(struct mt_device *td)
>> static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
>> {
>> struct mt_device *td = hid_get_drvdata(hdev);
>> - char *name = kstrdup(hdev->name, GFP_KERNEL);
>> -
>> - if (name)
>> - hi->input->name = name;
>> + char *name;
>> + const char *suffix = NULL;
>>
>> if (hi->report->id == td->mt_report_id)
>> mt_touch_input_configured(hdev, hi);
>>
>> - if (hi->report->id == td->pen_report_id)
>> + if (hi->report->field[0]->physical == HID_DG_STYLUS) {
>> + suffix = "Pen";
>> mt_pen_input_configured(hdev, hi);
>> + }
>> +
>> + if (suffix) {
>> + name = devm_kzalloc(&hi->input->dev,
>> + strlen(hdev->name) + strlen(suffix) + 2,
>> + GFP_KERNEL);
>> + if (name) {
>> + sprintf(name, "%s %s", hdev->name, suffix);
>> + hi->input->name = name;
>> + }
>> + }
>> }
>>
>> static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> @@ -945,7 +937,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> int ret, i;
>> struct mt_device *td;
>> struct mt_class *mtclass = mt_classes; /* MT_CLS_DEFAULT */
>> - struct hid_input *hi;
>>
>> for (i = 0; mt_classes[i].name ; i++) {
>> if (id->driver_data == mt_classes[i].name) {
>> @@ -967,7 +958,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> hdev->quirks |= HID_QUIRK_MULTI_INPUT;
>> hdev->quirks |= HID_QUIRK_NO_EMPTY_INPUT;
>>
>> - td = kzalloc(sizeof(struct mt_device), GFP_KERNEL);
>> + td = devm_kzalloc(&hdev->dev, sizeof(struct mt_device), GFP_KERNEL);
>> if (!td) {
>> dev_err(&hdev->dev, "cannot allocate multitouch data\n");
>> return -ENOMEM;
>> @@ -980,11 +971,11 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> td->pen_report_id = -1;
>> hid_set_drvdata(hdev, td);
>>
>> - td->fields = kzalloc(sizeof(struct mt_fields), GFP_KERNEL);
>> + td->fields = devm_kzalloc(&hdev->dev, sizeof(struct mt_fields),
>> + GFP_KERNEL);
>> if (!td->fields) {
>> dev_err(&hdev->dev, "cannot allocate multitouch fields data\n");
>> - ret = -ENOMEM;
>> - goto fail;
>> + return -ENOMEM;
>> }
>>
>> if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
>> @@ -992,29 +983,22 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>
>> ret = hid_parse(hdev);
>> if (ret != 0)
>> - goto fail;
>> + return ret;
>>
>> ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>> if (ret)
>> - goto hid_fail;
>> + return ret;
>>
>> ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
>>
>> mt_set_maxcontacts(hdev);
>> mt_set_input_mode(hdev);
>>
>> - kfree(td->fields);
>> + /* release .fields memory as it is not used anymore */
>> + devm_kfree(&hdev->dev, td->fields);
>> td->fields = NULL;
>>
>> return 0;
>> -
>> -hid_fail:
>> - list_for_each_entry(hi, &hdev->inputs, list)
>> - mt_free_input_name(hi);
>> -fail:
>> - kfree(td->fields);
>> - kfree(td);
>> - return ret;
>> }
>>
>> #ifdef CONFIG_PM
>> @@ -1039,17 +1023,8 @@ static int mt_resume(struct hid_device *hdev)
>>
>> static void mt_remove(struct hid_device *hdev)
>> {
>> - struct mt_device *td = hid_get_drvdata(hdev);
>> - struct hid_input *hi;
>> -
>> sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
>> - list_for_each_entry(hi, &hdev->inputs, list)
>> - mt_free_input_name(hi);
>> -
>> hid_hw_stop(hdev);
>> -
>> - kfree(td);
>> - hid_set_drvdata(hdev, NULL);
>> }
>>
>> static const struct hid_device_id mt_devices[] = {
>> --
>> 1.8.3.1
>>
>
>
>
> --
> With Best Regards,
> Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 3/3] Input: omap-keypad: Clear interrupts on open
From: Felipe Balbi @ 2013-07-24 16:06 UTC (permalink / raw)
To: Illia Smyrnov
Cc: Dmitry Torokhov, Felipe Balbi, linux-input, linux-kernel,
linux-omap
In-Reply-To: <1374681293-5320-4-git-send-email-illia.smyrnov@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]
Hi,
On Wed, Jul 24, 2013 at 06:54:53PM +0300, Illia Smyrnov wrote:
> Clear interrupts when open keypad.
>
> According to TRM, the recommended way for keyboard controller
> initialization is clear the interrupt-status register, then set up
> certain keyboard events for generating an interrupt request and
> set up expected source of wake-up event that generates a wake-up
> request.
>
> Signed-off-by: Illia Smyrnov <illia.smyrnov@ti.com>
> ---
> drivers/input/keyboard/omap4-keypad.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
> index b876a0d..0244262 100644
> --- a/drivers/input/keyboard/omap4-keypad.c
> +++ b/drivers/input/keyboard/omap4-keypad.c
> @@ -185,8 +185,9 @@ static int omap4_keypad_open(struct input_dev *input)
> (OMAP4_VAL_PVT << OMAP4_DEF_CTRL_PTV_SHIFT));
> kbd_writel(keypad_data, OMAP4_KBD_DEBOUNCINGTIME,
> OMAP4_VAL_DEBOUNCINGTIME);
> + /* clear pending interrupts */
> kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
> - OMAP4_VAL_IRQDISABLE);
> + kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
> kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
> OMAP4_DEF_IRQENABLE_EVENTEN |
> OMAP4_DEF_IRQENABLE_LONGKEY);
Reviewed-by: Felipe Balbi <balbi@ti.com>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH v3 1/3] Input: omap-keypad: Cleanup - use bitfiled instead of hardcoded values
From: Illia Smyrnov @ 2013-07-24 15:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Felipe Balbi, linux-input, linux-kernel, linux-omap
In-Reply-To: <1374681293-5320-1-git-send-email-illia.smyrnov@ti.com>
Use bitfiled instead of hardcoded values to set KBD_CTRL, use BIT macro,
remove unused defines.
Signed-off-by: Illia Smyrnov <illia.smyrnov@ti.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
---
drivers/input/keyboard/omap4-keypad.c | 25 +++++++++++--------------
1 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index f4aa53a..c727548 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -53,21 +53,17 @@
#define OMAP4_KBD_FULLCODE63_32 0x48
/* OMAP4 bit definitions */
-#define OMAP4_DEF_IRQENABLE_EVENTEN (1 << 0)
-#define OMAP4_DEF_IRQENABLE_LONGKEY (1 << 1)
-#define OMAP4_DEF_IRQENABLE_TIMEOUTEN (1 << 2)
-#define OMAP4_DEF_WUP_EVENT_ENA (1 << 0)
-#define OMAP4_DEF_WUP_LONG_KEY_ENA (1 << 1)
-#define OMAP4_DEF_CTRL_NOSOFTMODE (1 << 1)
-#define OMAP4_DEF_CTRLPTVVALUE (1 << 2)
-#define OMAP4_DEF_CTRLPTV (1 << 1)
+#define OMAP4_DEF_IRQENABLE_EVENTEN BIT(0)
+#define OMAP4_DEF_IRQENABLE_LONGKEY BIT(1)
+#define OMAP4_DEF_WUP_EVENT_ENA BIT(0)
+#define OMAP4_DEF_WUP_LONG_KEY_ENA BIT(1)
+#define OMAP4_DEF_CTRL_NOSOFTMODE BIT(1)
+#define OMAP4_DEF_CTRL_PTV_SHIFT 2
/* OMAP4 values */
-#define OMAP4_VAL_IRQDISABLE 0x00
-#define OMAP4_VAL_DEBOUNCINGTIME 0x07
-#define OMAP4_VAL_FUNCTIONALCFG 0x1E
-
-#define OMAP4_MASK_IRQSTATUSDISABLE 0xFFFF
+#define OMAP4_VAL_IRQDISABLE 0x0
+#define OMAP4_VAL_DEBOUNCINGTIME 0x7
+#define OMAP4_VAL_PVT 0x7
enum {
KBD_REVISION_OMAP4 = 0,
@@ -175,7 +171,8 @@ static int omap4_keypad_open(struct input_dev *input)
disable_irq(keypad_data->irq);
kbd_writel(keypad_data, OMAP4_KBD_CTRL,
- OMAP4_VAL_FUNCTIONALCFG);
+ OMAP4_DEF_CTRL_NOSOFTMODE |
+ (OMAP4_VAL_PVT << OMAP4_DEF_CTRL_PTV_SHIFT));
kbd_writel(keypad_data, OMAP4_KBD_DEBOUNCINGTIME,
OMAP4_VAL_DEBOUNCINGTIME);
kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 3/3] Input: omap-keypad: Clear interrupts on open
From: Illia Smyrnov @ 2013-07-24 15:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Felipe Balbi, linux-input, linux-kernel, linux-omap
In-Reply-To: <1374681293-5320-1-git-send-email-illia.smyrnov@ti.com>
Clear interrupts when open keypad.
According to TRM, the recommended way for keyboard controller
initialization is clear the interrupt-status register, then set up
certain keyboard events for generating an interrupt request and
set up expected source of wake-up event that generates a wake-up
request.
Signed-off-by: Illia Smyrnov <illia.smyrnov@ti.com>
---
drivers/input/keyboard/omap4-keypad.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index b876a0d..0244262 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -185,8 +185,9 @@ static int omap4_keypad_open(struct input_dev *input)
(OMAP4_VAL_PVT << OMAP4_DEF_CTRL_PTV_SHIFT));
kbd_writel(keypad_data, OMAP4_KBD_DEBOUNCINGTIME,
OMAP4_VAL_DEBOUNCINGTIME);
+ /* clear pending interrupts */
kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
- OMAP4_VAL_IRQDISABLE);
+ kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
OMAP4_DEF_IRQENABLE_EVENTEN |
OMAP4_DEF_IRQENABLE_LONGKEY);
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 2/3] Input: omap-keypad: Convert to threaded IRQ
From: Illia Smyrnov @ 2013-07-24 15:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Felipe Balbi, linux-input, linux-kernel, linux-omap
In-Reply-To: <1374681293-5320-1-git-send-email-illia.smyrnov@ti.com>
Convert to use threaded IRQ.
Cc: Felipe Balbi <balbi@ti.com>
Signed-off-by: Illia Smyrnov <illia.smyrnov@ti.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
---
drivers/input/keyboard/omap4-keypad.c | 29 ++++++++++++++++++++---------
1 files changed, 20 insertions(+), 9 deletions(-)
diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index c727548..b876a0d 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -112,8 +112,22 @@ static void kbd_write_irqreg(struct omap4_keypad *keypad_data,
}
-/* Interrupt handler */
-static irqreturn_t omap4_keypad_interrupt(int irq, void *dev_id)
+/* Interrupt handlers */
+static irqreturn_t omap4_keypad_irq_handler(int irq, void *dev_id)
+{
+ struct omap4_keypad *keypad_data = dev_id;
+
+ if (kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)) {
+ /* Disable interrupts */
+ kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
+ OMAP4_VAL_IRQDISABLE);
+ return IRQ_WAKE_THREAD;
+ }
+
+ return IRQ_NONE;
+}
+
+static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id)
{
struct omap4_keypad *keypad_data = dev_id;
struct input_dev *input_dev = keypad_data->input;
@@ -121,10 +135,6 @@ static irqreturn_t omap4_keypad_interrupt(int irq, void *dev_id)
unsigned int col, row, code, changed;
u32 *new_state = (u32 *) key_state;
- /* Disable interrupts */
- kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
- OMAP4_VAL_IRQDISABLE);
-
*new_state = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE31_0);
*(new_state + 1) = kbd_readl(keypad_data, OMAP4_KBD_FULLCODE63_32);
@@ -360,9 +370,10 @@ static int omap4_keypad_probe(struct platform_device *pdev)
goto err_free_keymap;
}
- error = request_irq(keypad_data->irq, omap4_keypad_interrupt,
- IRQF_TRIGGER_RISING,
- "omap4-keypad", keypad_data);
+ error = request_threaded_irq(keypad_data->irq, omap4_keypad_irq_handler,
+ omap4_keypad_irq_thread_fn,
+ IRQF_TRIGGER_RISING,
+ "omap4-keypad", keypad_data);
if (error) {
dev_err(&pdev->dev, "failed to register interrupt\n");
goto err_free_input;
--
1.7.0.4
^ permalink raw reply related
* [PATCH v3 0/3] Input: omap-keypad: Convert to threaded IRQ and cleanup
From: Illia Smyrnov @ 2013-07-24 15:54 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Felipe Balbi, linux-input, linux-kernel, linux-omap
Replace unclear hardcoded values with bit field, convert to threaded IRQ and
clear interrupts when open the keypad.
Based on top of v3.11-rc2.
Tested on OMAP4 SDP.
Illia Smyrnov (3):
Input: omap-keypad: Cleanup - use bitfiled instead of hardcoded
values
Input: omap-keypad: Convert to threaded IRQ
Input: omap-keypad: Clear interrupts on open
drivers/input/keyboard/omap4-keypad.c | 57 +++++++++++++++++++--------------
1 files changed, 33 insertions(+), 24 deletions(-)
^ permalink raw reply
* Re: [PATCH 2/2] HID: multitouch: devm conversion
From: Andy Shevchenko @ 2013-07-24 15:44 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Stephane Chatty,
linux-input, linux-kernel@vger.kernel.org
In-Reply-To: <1374679746-12566-3-git-send-email-benjamin.tissoires@redhat.com>
On Wed, Jul 24, 2013 at 6:29 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> HID special drivers can use safely the devres API.
> Use it to remove 25 lines of code and to clean up a little the error paths.
>
> Besides the pur kzalloc -> devm_kzalloc conversions, I changed the
Typo 'pur' -> 'put'
> place of the allocation of the new name. Doing this right in
> mt_input_configured() removes the kstrdup call which was not very helpful
> and the new way is simpler to understand (and to debug).
Thanks for the patch! I'm sorry I didn't find time to do this by myself.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/hid-multitouch.c | 71 ++++++++++++++------------------------------
> 1 file changed, 23 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index cb0e361..0fe00e2 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -261,17 +261,6 @@ static struct mt_class mt_classes[] = {
> { }
> };
>
> -static void mt_free_input_name(struct hid_input *hi)
> -{
> - struct hid_device *hdev = hi->report->device;
> - const char *name = hi->input->name;
> -
> - if (name != hdev->name) {
> - hi->input->name = hdev->name;
> - kfree(name);
> - }
> -}
> -
> static ssize_t mt_show_quirks(struct device *dev,
> struct device_attribute *attr,
> char *buf)
> @@ -415,13 +404,6 @@ static void mt_pen_report(struct hid_device *hid, struct hid_report *report)
> static void mt_pen_input_configured(struct hid_device *hdev,
> struct hid_input *hi)
> {
> - char *name = kzalloc(strlen(hi->input->name) + 5, GFP_KERNEL);
> - if (name) {
> - sprintf(name, "%s Pen", hi->input->name);
> - mt_free_input_name(hi);
> - hi->input->name = name;
> - }
> -
> /* force BTN_STYLUS to allow tablet matching in udev */
> __set_bit(BTN_STYLUS, hi->input->keybit);
> }
> @@ -928,16 +910,26 @@ static void mt_post_parse(struct mt_device *td)
> static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
> {
> struct mt_device *td = hid_get_drvdata(hdev);
> - char *name = kstrdup(hdev->name, GFP_KERNEL);
> -
> - if (name)
> - hi->input->name = name;
> + char *name;
> + const char *suffix = NULL;
>
> if (hi->report->id == td->mt_report_id)
> mt_touch_input_configured(hdev, hi);
>
> - if (hi->report->id == td->pen_report_id)
> + if (hi->report->field[0]->physical == HID_DG_STYLUS) {
> + suffix = "Pen";
> mt_pen_input_configured(hdev, hi);
> + }
> +
> + if (suffix) {
> + name = devm_kzalloc(&hi->input->dev,
> + strlen(hdev->name) + strlen(suffix) + 2,
> + GFP_KERNEL);
> + if (name) {
> + sprintf(name, "%s %s", hdev->name, suffix);
> + hi->input->name = name;
> + }
> + }
> }
>
> static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> @@ -945,7 +937,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> int ret, i;
> struct mt_device *td;
> struct mt_class *mtclass = mt_classes; /* MT_CLS_DEFAULT */
> - struct hid_input *hi;
>
> for (i = 0; mt_classes[i].name ; i++) {
> if (id->driver_data == mt_classes[i].name) {
> @@ -967,7 +958,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> hdev->quirks |= HID_QUIRK_MULTI_INPUT;
> hdev->quirks |= HID_QUIRK_NO_EMPTY_INPUT;
>
> - td = kzalloc(sizeof(struct mt_device), GFP_KERNEL);
> + td = devm_kzalloc(&hdev->dev, sizeof(struct mt_device), GFP_KERNEL);
> if (!td) {
> dev_err(&hdev->dev, "cannot allocate multitouch data\n");
> return -ENOMEM;
> @@ -980,11 +971,11 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
> td->pen_report_id = -1;
> hid_set_drvdata(hdev, td);
>
> - td->fields = kzalloc(sizeof(struct mt_fields), GFP_KERNEL);
> + td->fields = devm_kzalloc(&hdev->dev, sizeof(struct mt_fields),
> + GFP_KERNEL);
> if (!td->fields) {
> dev_err(&hdev->dev, "cannot allocate multitouch fields data\n");
> - ret = -ENOMEM;
> - goto fail;
> + return -ENOMEM;
> }
>
> if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
> @@ -992,29 +983,22 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
>
> ret = hid_parse(hdev);
> if (ret != 0)
> - goto fail;
> + return ret;
>
> ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> if (ret)
> - goto hid_fail;
> + return ret;
>
> ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
>
> mt_set_maxcontacts(hdev);
> mt_set_input_mode(hdev);
>
> - kfree(td->fields);
> + /* release .fields memory as it is not used anymore */
> + devm_kfree(&hdev->dev, td->fields);
> td->fields = NULL;
>
> return 0;
> -
> -hid_fail:
> - list_for_each_entry(hi, &hdev->inputs, list)
> - mt_free_input_name(hi);
> -fail:
> - kfree(td->fields);
> - kfree(td);
> - return ret;
> }
>
> #ifdef CONFIG_PM
> @@ -1039,17 +1023,8 @@ static int mt_resume(struct hid_device *hdev)
>
> static void mt_remove(struct hid_device *hdev)
> {
> - struct mt_device *td = hid_get_drvdata(hdev);
> - struct hid_input *hi;
> -
> sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
> - list_for_each_entry(hi, &hdev->inputs, list)
> - mt_free_input_name(hi);
> -
> hid_hw_stop(hdev);
> -
> - kfree(td);
> - hid_set_drvdata(hdev, NULL);
> }
>
> static const struct hid_device_id mt_devices[] = {
> --
> 1.8.3.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 1/2] HID: trivial devm conversion for special hid drivers
From: Benjamin Tissoires @ 2013-07-24 15:43 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Stephane Chatty,
linux-input, linux-kernel@vger.kernel.org
In-Reply-To: <CAHp75Vcr1MP7eeKHK3pwh49R5Yy0-guQHBJjZ+WXyiR5MT+ZBg@mail.gmail.com>
On Wed, Jul 24, 2013 at 5:37 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Wed, Jul 24, 2013 at 6:29 PM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>> It is safe to use devres allocation within the hid subsystem:
>> - the devres release is called _after_ the call to .remove(), meaning
>> that no freed pointers will exists while removing the device
>> - if a .probe() fails, devres releases all the allocated ressources
>> before going to the next driver: there will not be ghost ressources
>> attached to a hid device if several drivers are probed.
>>
>> Given that, we can clean up a little some of the HID drivers. These ones
>> are trivial:
>> - there is only one kzalloc in the driver
>> - the .remove() callback contains only one kfree on top of hid_hw_stop()
>> - the error path in the probe is easy enough to be manually checked
>
> Thanks for the patch! I'm sorry I didn't find time to do what I was
> talking about last time.
no problems :)
>
> Few comments below.
>
>> --- a/drivers/hid/hid-a4tech.c
>> +++ b/drivers/hid/hid-a4tech.c
>
>> @@ -104,29 +103,16 @@ static int a4_probe(struct hid_device *hdev, const struct hid_device_id *id)
>> ret = hid_parse(hdev);
>> if (ret) {
>> hid_err(hdev, "parse failed\n");
>> - goto err_free;
>> + return ret;
>> }
>>
>> ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
>> - if (ret) {
>> + if (ret)
>> hid_err(hdev, "hw start failed\n");
>> - goto err_free;
>> - }
>>
>> - return 0;
>
> Isn't it better to leave explicit return 0? I think it would be fool
> proof in case someone wants to add anything in the middle.
yes, it might be. At least that's what I've done with the other drivers...
>
>> -err_free:
>> - kfree(a4);
>> return ret;
>> }
>
>> -static void a4_remove(struct hid_device *hdev)
>> -{
>> - struct a4tech_sc *a4 = hid_get_drvdata(hdev);
>> -
>> - hid_hw_stop(hdev);
>
> Is it safe to remove this call?
> This question is the same for all patched drivers.
It is. Once this call is removed, we use the in-core remove path,
which calls hid_hw_stop().
Thanks for the review.
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: add DT bindings
From: Benjamin Tissoires @ 2013-07-24 15:39 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, Grant Likely, Rob Herring, devicetree-discuss,
linux-input, linux-kernel@vger.kernel.org
In-Reply-To: <alpine.LNX.2.00.1306181151030.6750@pobox.suse.cz>
On Tue, Jun 18, 2013 at 11:51 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Thu, 13 Jun 2013, Benjamin Tissoires wrote:
>
>> Add device tree based support for HID over I2C devices.
>>
>> Tested on an Odroid-X board with a Synaptics touchpad.
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> ---
>>
>> Hi guys,
>>
>> well, as the commit message says, this is the DT binding for HID over I2C.
>> I honestly don't know if it will be used besides me, but it may help others
>> with a DT based board.
>> As the spec is for ACPI only, I had no specifications regarding the DT names. So
>> these names can be changed if you think they are bad.
>>
>> I also created a new bindings directory in the devicetree doc to reflect the
>> split we have between input and hid. However, if the DT experts prefer having
>> it under input, I'm fine with that.
>>
>> Cheers,
>> Benjamin
>>
>> .../devicetree/bindings/hid/hid-over-i2c.txt | 28 ++++++++++++++
>> drivers/hid/i2c-hid/i2c-hid.c | 44 +++++++++++++++++++++-
>> include/linux/i2c/i2c-hid.h | 3 +-
>> 3 files changed, 73 insertions(+), 2 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/hid/hid-over-i2c.txt
>>
>> diff --git a/Documentation/devicetree/bindings/hid/hid-over-i2c.txt b/Documentation/devicetree/bindings/hid/hid-over-i2c.txt
>
> DT folks, any objections to this, please?
>
> If not, I'd like to apply this for 3.11 into my tree.
>
DT folks, ping???
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH 1/2] HID: trivial devm conversion for special hid drivers
From: Andy Shevchenko @ 2013-07-24 15:37 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Stephane Chatty,
linux-input, linux-kernel@vger.kernel.org
In-Reply-To: <1374679746-12566-2-git-send-email-benjamin.tissoires@redhat.com>
On Wed, Jul 24, 2013 at 6:29 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> It is safe to use devres allocation within the hid subsystem:
> - the devres release is called _after_ the call to .remove(), meaning
> that no freed pointers will exists while removing the device
> - if a .probe() fails, devres releases all the allocated ressources
> before going to the next driver: there will not be ghost ressources
> attached to a hid device if several drivers are probed.
>
> Given that, we can clean up a little some of the HID drivers. These ones
> are trivial:
> - there is only one kzalloc in the driver
> - the .remove() callback contains only one kfree on top of hid_hw_stop()
> - the error path in the probe is easy enough to be manually checked
Thanks for the patch! I'm sorry I didn't find time to do what I was
talking about last time.
Few comments below.
> --- a/drivers/hid/hid-a4tech.c
> +++ b/drivers/hid/hid-a4tech.c
> @@ -104,29 +103,16 @@ static int a4_probe(struct hid_device *hdev, const struct hid_device_id *id)
> ret = hid_parse(hdev);
> if (ret) {
> hid_err(hdev, "parse failed\n");
> - goto err_free;
> + return ret;
> }
>
> ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> - if (ret) {
> + if (ret)
> hid_err(hdev, "hw start failed\n");
> - goto err_free;
> - }
>
> - return 0;
Isn't it better to leave explicit return 0? I think it would be fool
proof in case someone wants to add anything in the middle.
> -err_free:
> - kfree(a4);
> return ret;
> }
> -static void a4_remove(struct hid_device *hdev)
> -{
> - struct a4tech_sc *a4 = hid_get_drvdata(hdev);
> -
> - hid_hw_stop(hdev);
Is it safe to remove this call?
This question is the same for all patched drivers.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH 1/2] HID: trivial devm conversion for special hid drivers
From: Benjamin Tissoires @ 2013-07-24 15:29 UTC (permalink / raw)
To: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Andy Shevchenko,
Stephane Chatty, linux-input, linux-kernel
In-Reply-To: <1374679746-12566-1-git-send-email-benjamin.tissoires@redhat.com>
It is safe to use devres allocation within the hid subsystem:
- the devres release is called _after_ the call to .remove(), meaning
that no freed pointers will exists while removing the device
- if a .probe() fails, devres releases all the allocated ressources
before going to the next driver: there will not be ghost ressources
attached to a hid device if several drivers are probed.
Given that, we can clean up a little some of the HID drivers. These ones
are trivial:
- there is only one kzalloc in the driver
- the .remove() callback contains only one kfree on top of hid_hw_stop()
- the error path in the probe is easy enough to be manually checked
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-a4tech.c | 23 ++++-------------------
drivers/hid/hid-apple.c | 16 +++-------------
drivers/hid/hid-magicmouse.c | 17 +++--------------
drivers/hid/hid-sony.c | 9 +++------
drivers/hid/hid-zydacron.c | 19 +++----------------
5 files changed, 16 insertions(+), 68 deletions(-)
diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
index 7c5507e..8d4e999 100644
--- a/drivers/hid/hid-a4tech.c
+++ b/drivers/hid/hid-a4tech.c
@@ -90,11 +90,10 @@ static int a4_probe(struct hid_device *hdev, const struct hid_device_id *id)
struct a4tech_sc *a4;
int ret;
- a4 = kzalloc(sizeof(*a4), GFP_KERNEL);
+ a4 = devm_kzalloc(&hdev->dev, sizeof(*a4), GFP_KERNEL);
if (a4 == NULL) {
hid_err(hdev, "can't alloc device descriptor\n");
- ret = -ENOMEM;
- goto err_free;
+ return -ENOMEM;
}
a4->quirks = id->driver_data;
@@ -104,29 +103,16 @@ static int a4_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
- goto err_free;
+ return ret;
}
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- if (ret) {
+ if (ret)
hid_err(hdev, "hw start failed\n");
- goto err_free;
- }
- return 0;
-err_free:
- kfree(a4);
return ret;
}
-static void a4_remove(struct hid_device *hdev)
-{
- struct a4tech_sc *a4 = hid_get_drvdata(hdev);
-
- hid_hw_stop(hdev);
- kfree(a4);
-}
-
static const struct hid_device_id a4_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_A4TECH, USB_DEVICE_ID_A4TECH_WCP32PU),
.driver_data = A4_2WHEEL_MOUSE_HACK_7 },
@@ -144,7 +130,6 @@ static struct hid_driver a4_driver = {
.input_mapped = a4_input_mapped,
.event = a4_event,
.probe = a4_probe,
- .remove = a4_remove,
};
module_hid_driver(a4_driver);
diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
index c7710b5..881cf7b 100644
--- a/drivers/hid/hid-apple.c
+++ b/drivers/hid/hid-apple.c
@@ -349,7 +349,7 @@ static int apple_probe(struct hid_device *hdev,
unsigned int connect_mask = HID_CONNECT_DEFAULT;
int ret;
- asc = kzalloc(sizeof(*asc), GFP_KERNEL);
+ asc = devm_kzalloc(&hdev->dev, sizeof(*asc), GFP_KERNEL);
if (asc == NULL) {
hid_err(hdev, "can't alloc apple descriptor\n");
return -ENOMEM;
@@ -362,7 +362,7 @@ static int apple_probe(struct hid_device *hdev,
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
- goto err_free;
+ return ret;
}
if (quirks & APPLE_HIDDEV)
@@ -373,19 +373,10 @@ static int apple_probe(struct hid_device *hdev,
ret = hid_hw_start(hdev, connect_mask);
if (ret) {
hid_err(hdev, "hw start failed\n");
- goto err_free;
+ return ret;
}
return 0;
-err_free:
- kfree(asc);
- return ret;
-}
-
-static void apple_remove(struct hid_device *hdev)
-{
- hid_hw_stop(hdev);
- kfree(hid_get_drvdata(hdev));
}
static const struct hid_device_id apple_devices[] = {
@@ -551,7 +542,6 @@ static struct hid_driver apple_driver = {
.id_table = apple_devices,
.report_fixup = apple_report_fixup,
.probe = apple_probe,
- .remove = apple_remove,
.event = apple_event,
.input_mapping = apple_input_mapping,
.input_mapped = apple_input_mapped,
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 5bc3734..d393eb7 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -484,7 +484,7 @@ static int magicmouse_probe(struct hid_device *hdev,
struct hid_report *report;
int ret;
- msc = kzalloc(sizeof(*msc), GFP_KERNEL);
+ msc = devm_kzalloc(&hdev->dev, sizeof(*msc), GFP_KERNEL);
if (msc == NULL) {
hid_err(hdev, "can't alloc magicmouse descriptor\n");
return -ENOMEM;
@@ -498,13 +498,13 @@ static int magicmouse_probe(struct hid_device *hdev,
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "magicmouse hid parse failed\n");
- goto err_free;
+ return ret;
}
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "magicmouse hw start failed\n");
- goto err_free;
+ return ret;
}
if (!msc->input) {
@@ -548,19 +548,9 @@ static int magicmouse_probe(struct hid_device *hdev,
return 0;
err_stop_hw:
hid_hw_stop(hdev);
-err_free:
- kfree(msc);
return ret;
}
-static void magicmouse_remove(struct hid_device *hdev)
-{
- struct magicmouse_sc *msc = hid_get_drvdata(hdev);
-
- hid_hw_stop(hdev);
- kfree(msc);
-}
-
static const struct hid_device_id magic_mice[] = {
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_APPLE,
USB_DEVICE_ID_APPLE_MAGICMOUSE), .driver_data = 0 },
@@ -574,7 +564,6 @@ static struct hid_driver magicmouse_driver = {
.name = "magicmouse",
.id_table = magic_mice,
.probe = magicmouse_probe,
- .remove = magicmouse_remove,
.raw_event = magicmouse_raw_event,
.input_mapping = magicmouse_input_mapping,
.input_configured = magicmouse_input_configured,
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 87fbe29..30dbb6b 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -624,7 +624,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
struct sony_sc *sc;
unsigned int connect_mask = HID_CONNECT_DEFAULT;
- sc = kzalloc(sizeof(*sc), GFP_KERNEL);
+ sc = devm_kzalloc(&hdev->dev, sizeof(*sc), GFP_KERNEL);
if (sc == NULL) {
hid_err(hdev, "can't alloc sony descriptor\n");
return -ENOMEM;
@@ -636,7 +636,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
- goto err_free;
+ return ret;
}
if (sc->quirks & VAIO_RDESC_CONSTANT)
@@ -649,7 +649,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_hw_start(hdev, connect_mask);
if (ret) {
hid_err(hdev, "hw start failed\n");
- goto err_free;
+ return ret;
}
if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
@@ -669,8 +669,6 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
return 0;
err_stop:
hid_hw_stop(hdev);
-err_free:
- kfree(sc);
return ret;
}
@@ -682,7 +680,6 @@ static void sony_remove(struct hid_device *hdev)
buzz_remove(hdev);
hid_hw_stop(hdev);
- kfree(sc);
}
static const struct hid_device_id sony_devices[] = {
diff --git a/drivers/hid/hid-zydacron.c b/drivers/hid/hid-zydacron.c
index e4cddec..1a660bd 100644
--- a/drivers/hid/hid-zydacron.c
+++ b/drivers/hid/hid-zydacron.c
@@ -169,7 +169,7 @@ static int zc_probe(struct hid_device *hdev, const struct hid_device_id *id)
int ret;
struct zc_device *zc;
- zc = kzalloc(sizeof(*zc), GFP_KERNEL);
+ zc = devm_kzalloc(&hdev->dev, sizeof(*zc), GFP_KERNEL);
if (zc == NULL) {
hid_err(hdev, "can't alloc descriptor\n");
return -ENOMEM;
@@ -180,28 +180,16 @@ static int zc_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
- goto err_free;
+ return ret;
}
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret) {
hid_err(hdev, "hw start failed\n");
- goto err_free;
+ return ret;
}
return 0;
-err_free:
- kfree(zc);
-
- return ret;
-}
-
-static void zc_remove(struct hid_device *hdev)
-{
- struct zc_device *zc = hid_get_drvdata(hdev);
-
- hid_hw_stop(hdev);
- kfree(zc);
}
static const struct hid_device_id zc_devices[] = {
@@ -217,7 +205,6 @@ static struct hid_driver zc_driver = {
.input_mapping = zc_input_mapping,
.raw_event = zc_raw_event,
.probe = zc_probe,
- .remove = zc_remove,
};
module_hid_driver(zc_driver);
--
1.8.3.1
^ permalink raw reply related
* [PATCH 0/2] HID special drivers converted to devres API
From: Benjamin Tissoires @ 2013-07-24 15:29 UTC (permalink / raw)
To: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Andy Shevchenko,
Stephane Chatty, linux-input, linux-kernel
Hi Jiri,
It's been a long time since I told you that I was about to work on the devres API...
So here are the first patches.
My deductions are that it is safe to run devm_* functions within the hid subsystem.
With this, we can simplify a little the error path of some drivers. The gain
is more visible for hid-multitouch as the function mt_free_input_name() can
be entirely dropped.
Cheers,
Benjamin
Benjamin Tissoires (2):
HID: trivial devm conversion for special hid drivers
HID: multitouch: devm conversion
drivers/hid/hid-a4tech.c | 23 +++-----------
drivers/hid/hid-apple.c | 16 ++--------
drivers/hid/hid-magicmouse.c | 17 ++---------
drivers/hid/hid-multitouch.c | 71 ++++++++++++++------------------------------
drivers/hid/hid-sony.c | 9 ++----
drivers/hid/hid-zydacron.c | 19 ++----------
6 files changed, 39 insertions(+), 116 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH 2/2] HID: multitouch: devm conversion
From: Benjamin Tissoires @ 2013-07-24 15:29 UTC (permalink / raw)
To: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Andy Shevchenko,
Stephane Chatty, linux-input, linux-kernel
In-Reply-To: <1374679746-12566-1-git-send-email-benjamin.tissoires@redhat.com>
HID special drivers can use safely the devres API.
Use it to remove 25 lines of code and to clean up a little the error paths.
Besides the pur kzalloc -> devm_kzalloc conversions, I changed the
place of the allocation of the new name. Doing this right in
mt_input_configured() removes the kstrdup call which was not very helpful
and the new way is simpler to understand (and to debug).
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-multitouch.c | 71 ++++++++++++++------------------------------
1 file changed, 23 insertions(+), 48 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index cb0e361..0fe00e2 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -261,17 +261,6 @@ static struct mt_class mt_classes[] = {
{ }
};
-static void mt_free_input_name(struct hid_input *hi)
-{
- struct hid_device *hdev = hi->report->device;
- const char *name = hi->input->name;
-
- if (name != hdev->name) {
- hi->input->name = hdev->name;
- kfree(name);
- }
-}
-
static ssize_t mt_show_quirks(struct device *dev,
struct device_attribute *attr,
char *buf)
@@ -415,13 +404,6 @@ static void mt_pen_report(struct hid_device *hid, struct hid_report *report)
static void mt_pen_input_configured(struct hid_device *hdev,
struct hid_input *hi)
{
- char *name = kzalloc(strlen(hi->input->name) + 5, GFP_KERNEL);
- if (name) {
- sprintf(name, "%s Pen", hi->input->name);
- mt_free_input_name(hi);
- hi->input->name = name;
- }
-
/* force BTN_STYLUS to allow tablet matching in udev */
__set_bit(BTN_STYLUS, hi->input->keybit);
}
@@ -928,16 +910,26 @@ static void mt_post_parse(struct mt_device *td)
static void mt_input_configured(struct hid_device *hdev, struct hid_input *hi)
{
struct mt_device *td = hid_get_drvdata(hdev);
- char *name = kstrdup(hdev->name, GFP_KERNEL);
-
- if (name)
- hi->input->name = name;
+ char *name;
+ const char *suffix = NULL;
if (hi->report->id == td->mt_report_id)
mt_touch_input_configured(hdev, hi);
- if (hi->report->id == td->pen_report_id)
+ if (hi->report->field[0]->physical == HID_DG_STYLUS) {
+ suffix = "Pen";
mt_pen_input_configured(hdev, hi);
+ }
+
+ if (suffix) {
+ name = devm_kzalloc(&hi->input->dev,
+ strlen(hdev->name) + strlen(suffix) + 2,
+ GFP_KERNEL);
+ if (name) {
+ sprintf(name, "%s %s", hdev->name, suffix);
+ hi->input->name = name;
+ }
+ }
}
static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
@@ -945,7 +937,6 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
int ret, i;
struct mt_device *td;
struct mt_class *mtclass = mt_classes; /* MT_CLS_DEFAULT */
- struct hid_input *hi;
for (i = 0; mt_classes[i].name ; i++) {
if (id->driver_data == mt_classes[i].name) {
@@ -967,7 +958,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
hdev->quirks |= HID_QUIRK_MULTI_INPUT;
hdev->quirks |= HID_QUIRK_NO_EMPTY_INPUT;
- td = kzalloc(sizeof(struct mt_device), GFP_KERNEL);
+ td = devm_kzalloc(&hdev->dev, sizeof(struct mt_device), GFP_KERNEL);
if (!td) {
dev_err(&hdev->dev, "cannot allocate multitouch data\n");
return -ENOMEM;
@@ -980,11 +971,11 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
td->pen_report_id = -1;
hid_set_drvdata(hdev, td);
- td->fields = kzalloc(sizeof(struct mt_fields), GFP_KERNEL);
+ td->fields = devm_kzalloc(&hdev->dev, sizeof(struct mt_fields),
+ GFP_KERNEL);
if (!td->fields) {
dev_err(&hdev->dev, "cannot allocate multitouch fields data\n");
- ret = -ENOMEM;
- goto fail;
+ return -ENOMEM;
}
if (id->vendor == HID_ANY_ID && id->product == HID_ANY_ID)
@@ -992,29 +983,22 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
ret = hid_parse(hdev);
if (ret != 0)
- goto fail;
+ return ret;
ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
if (ret)
- goto hid_fail;
+ return ret;
ret = sysfs_create_group(&hdev->dev.kobj, &mt_attribute_group);
mt_set_maxcontacts(hdev);
mt_set_input_mode(hdev);
- kfree(td->fields);
+ /* release .fields memory as it is not used anymore */
+ devm_kfree(&hdev->dev, td->fields);
td->fields = NULL;
return 0;
-
-hid_fail:
- list_for_each_entry(hi, &hdev->inputs, list)
- mt_free_input_name(hi);
-fail:
- kfree(td->fields);
- kfree(td);
- return ret;
}
#ifdef CONFIG_PM
@@ -1039,17 +1023,8 @@ static int mt_resume(struct hid_device *hdev)
static void mt_remove(struct hid_device *hdev)
{
- struct mt_device *td = hid_get_drvdata(hdev);
- struct hid_input *hi;
-
sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
- list_for_each_entry(hi, &hdev->inputs, list)
- mt_free_input_name(hi);
-
hid_hw_stop(hdev);
-
- kfree(td);
- hid_set_drvdata(hdev, NULL);
}
static const struct hid_device_id mt_devices[] = {
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] HID: multitouch: do not init reports for multitouch devices
From: Benjamin Tissoires @ 2013-07-24 15:01 UTC (permalink / raw)
To: Henrik Rydberg
Cc: Benjamin Tissoires, Jiri Kosina, Stephane Chatty, linux-input,
linux-kernel@vger.kernel.org
In-Reply-To: <20130719210430.GA31481@polaris.bitmath.org>
Hi Henrik,
On Fri, Jul 19, 2013 at 11:04 PM, <rydberg@euromail.se> wrote:
> Hi Benjamin,
>
>> >> Some multitouch screens do not like to be polled for input reports.
>> >> However, the Win8 spec says that all touches should be sent during
>> >> each report, making the initialization of reports unnecessary.
>> >> The Win7 spec is less precise, but we can safely assume that when
>> >> the module is loaded (at boot), no one is touching the screen.
>> >>
>> >> Add the quirk HID_QUIRK_NO_INIT_REPORTS so that we do not have to
>> >> introduce a quirk for each problematic device.
>> >
>> > I assume you have tested thoroughly for regressions? How about odd
>> > eGalax devices, for instance? Changes affecting existing hardware
>> > makes me nervous. Is it so bad to add this quirk on a per-device
>> > basis? Or perhaps turned on by default for win8 devices only?
>>
>> Aargh, I forgot the eGalax... (I don't have it anymore on my desk). I
>> was pretty confident because Win [7-8] is not doing any quirks for the
>> multitouch devices, and I had in mind that it did not asked for the
>> reports at startup (at least, I am sure about it for HID/I2C). I'm not
>> sure win 8 devices is a sufficient denominator, because this init
>> sequence is not mentioned anywhere in the Win 8 spec. It's true that
>> we are going to see fewer Win 7 devices, but I would say it's the
>> exact same problem for win 7 and 8. Moreover, asking this for Win 8
>> devices only will forces us to detect it in core before hid-multitouch
>> is loaded because the init reports is called before the parsing.
>
> We already branch on report specifics in hid_add_device(). Adding win8
> detection there is more or less what it was built for.
right. So I will send a following series to detect win8 multitouch
device in core.
>
>> If I capture the Win 7 & Win 8 initialization events and I observe
>> that they do not retrieve the reports, will it be sufficient as a
>> guarantee to include this patch even if it is not widely tested under
>> Linux?
Actually, the current quirk is not imitating Windows behavior. Windows
does not retrieve the input reports, but only the feature reports.
This scheme is also useful for us as the feature_mapping() callback
makes use of the ->value, and if we do not retrieve the feature
report, the field is left uninitialized.
I will also add an other quirk to retrieve only the features at init.
>
> We already have the usbhid quirks to handle odd cases, and we can add
> all sorts of generic detection during device add, so there really is
> no reason to risk regressions at all, is there?
Ok, so let's go for another patch series.. :)
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH v3.11-rc2] HID: sony: fix HID mapping for PS3 sixaxis controller
From: Jiri Kosina @ 2013-07-24 14:57 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <1374677587-8194-1-git-send-email-benjamin.tissoires@redhat.com>
On Wed, 24 Jul 2013, Benjamin Tissoires wrote:
> Commit f04d51404f51 (HID: driver for PS2/3 Buzz controllers) introduced
> an input_mapping() callback, but set the return value to -1 to all devices
> except the Buzz controllers. The result of this is that the Sixaxis input
> device is not populated, making it useless.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> Hi Jiri,
>
> I have since yesterday a Sixaxis controller on my desk, and I noticed that it
> was broken in v3.11.
Good catch, applied, thanks Benjamin.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH v3.11-rc2] HID: sony: fix HID mapping for PS3 sixaxis controller
From: Benjamin Tissoires @ 2013-07-24 14:53 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina, linux-input, linux-kernel
Commit f04d51404f51 (HID: driver for PS2/3 Buzz controllers) introduced
an input_mapping() callback, but set the return value to -1 to all devices
except the Buzz controllers. The result of this is that the Sixaxis input
device is not populated, making it useless.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
Hi Jiri,
I have since yesterday a Sixaxis controller on my desk, and I noticed that it
was broken in v3.11.
Cheers,
Benjamin
drivers/hid/hid-sony.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index ecbc749..87fbe29 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -369,7 +369,8 @@ static int sony_mapping(struct hid_device *hdev, struct hid_input *hi,
if (sc->quirks & PS3REMOTE)
return ps3remote_mapping(hdev, hi, field, usage, bit, max);
- return -1;
+ /* Let hid-core decide for the others */
+ return 0;
}
/*
--
1.8.3.1
^ permalink raw reply related
* [PATCH 07/27] drivers/input/serio: don't check resource with devm_ioremap_resource
From: Wolfram Sang @ 2013-07-23 18:01 UTC (permalink / raw)
To: linux-kernel; +Cc: Wolfram Sang, Dmitry Torokhov, linux-input
In-Reply-To: <1374602524-3398-1-git-send-email-wsa@the-dreams.de>
devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
---
Please apply via the subsystem-tree.
drivers/input/serio/olpc_apsp.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/input/serio/olpc_apsp.c b/drivers/input/serio/olpc_apsp.c
index 818aa46..51b1d40 100644
--- a/drivers/input/serio/olpc_apsp.c
+++ b/drivers/input/serio/olpc_apsp.c
@@ -183,9 +183,6 @@ static int olpc_apsp_probe(struct platform_device *pdev)
np = pdev->dev.of_node;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res)
- return -ENOENT;
-
priv->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(priv->base)) {
dev_err(&pdev->dev, "Failed to map WTM registers\n");
--
1.7.10.4
^ permalink raw reply related
* [PATCH 00/27] devm cleanup, part #1, take #3
From: Wolfram Sang @ 2013-07-23 18:01 UTC (permalink / raw)
To: linux-kernel
Cc: Wolfram Sang, alsa-devel, Bjorn Helgaas, Daniel Lezcano,
Dan Williams, David Woodhouse, devel, Dmitry Torokhov, dri-devel,
Eduardo Valentin, Eric Miao, Felipe Balbi, Giuseppe Cavallaro,
Grant Likely, Greg Kroah-Hartman, Haojian Zhuang,
James E.J. Bottomley, Jaroslav Kysela, Liam Girdwood,
Linus Walleij
Here is another bit of cleaning up the devm usage. It is again removing the
resource check with devm_ioremap_resource, because a) new drivers came in and
b) coccinelle had a bug and missed to find a couple of occasions. Unlike last
time, I think it is better if these patches go in via the subsystem trees to
reduce merge conflicts. And there is one driver which I fixed manually because
the original code needed some bigger update. All is based on v3.11-rc2 and the
branch can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git devm_no_resource_check
Other things which happened: I wanted to get rid of devm_request_and_ioremap,
luckily other people are already working on it. Hooray! I already sent a patch
series picking another low hanging fruit, i.e. drivers can skip devm_pinctrl
handling if they are using only the default pin setup. And not devm related,
there is still my proposal to rename INIT_COMPLETION to reinit_completion, that
probably needs some more persistence... Yay, so much to clean up \o/
Regards,
Wolfram
Wolfram Sang (27):
arch/mips/lantiq/xway: don't check resource with
devm_ioremap_resource
drivers/amba: don't check resource with devm_ioremap_resource
drivers/cpuidle: don't check resource with devm_ioremap_resource
drivers/dma: don't check resource with devm_ioremap_resource
drivers/gpu/host1x/drm: don't check resource with
devm_ioremap_resource
drivers/i2c/busses: don't check resource with devm_ioremap_resource
drivers/input/serio: don't check resource with devm_ioremap_resource
drivers/iommu: don't check resource with devm_ioremap_resource
drivers/media/platform: don't check resource with
devm_ioremap_resource
drivers/memory: don't check resource with devm_ioremap_resource
drivers/mtd/nand: don't check resource with devm_ioremap_resource
drivers/net/ethernet/stmicro/stmmac: don't check resource with
devm_ioremap_resource
drivers/pci/host: don't check resource with devm_ioremap_resource
drivers/pinctrl: don't check resource with devm_ioremap_resource
drivers/pwm: don't check resource with devm_ioremap_resource
drivers/scsi/ufs: don't check resource with devm_ioremap_resource
drivers/spi: don't check resource with devm_ioremap_resource
drivers/staging/imx-drm: don't check resource with
devm_ioremap_resource
drivers/usb/phy: don't check resource with devm_ioremap_resource
drivers/watchdog: don't check resource with devm_ioremap_resource
sound/soc/au1x: don't check resource with devm_ioremap_resource
sound/soc/cirrus: don't check resource with devm_ioremap_resource
sound/soc/nuc900: don't check resource with devm_ioremap_resource
sound/soc/pxa: don't check resource with devm_ioremap_resource
sound/soc/tegra: don't check resource with devm_ioremap_resource
sound/soc/txx9: don't check resource with devm_ioremap_resource
thermal: ti-bandgap: cleanup resource allocation
arch/mips/lantiq/xway/dma.c | 4 ----
drivers/amba/tegra-ahb.c | 2 --
drivers/cpuidle/cpuidle-kirkwood.c | 3 ---
drivers/dma/mmp_pdma.c | 3 ---
drivers/dma/mmp_tdma.c | 3 ---
drivers/gpu/host1x/drm/hdmi.c | 3 ---
drivers/i2c/busses/i2c-stu300.c | 3 ---
drivers/input/serio/olpc_apsp.c | 3 ---
drivers/iommu/tegra-smmu.c | 2 --
drivers/media/platform/coda.c | 5 -----
drivers/memory/tegra20-mc.c | 2 --
drivers/memory/tegra30-mc.c | 2 --
drivers/mtd/nand/mxc_nand.c | 5 -----
.../net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 ---
drivers/pci/host/pcie-designware.c | 12 ------------
drivers/pinctrl/pinctrl-imx.c | 3 ---
drivers/pinctrl/pinctrl-rockchip.c | 5 -----
drivers/pinctrl/pinctrl-u300.c | 3 ---
drivers/pwm/pwm-lpc32xx.c | 3 ---
drivers/pwm/pwm-renesas-tpu.c | 5 -----
drivers/scsi/ufs/ufshcd-pltfrm.c | 6 ------
drivers/spi/spi-bcm2835.c | 6 ------
drivers/staging/imx-drm/imx-tve.c | 5 -----
drivers/thermal/ti-soc-thermal/ti-bandgap.c | 20 ++++----------------
drivers/usb/phy/phy-rcar-usb.c | 5 -----
drivers/watchdog/nuc900_wdt.c | 5 -----
drivers/watchdog/ts72xx_wdt.c | 10 ----------
sound/soc/au1x/psc-ac97.c | 3 ---
sound/soc/cirrus/ep93xx-ac97.c | 3 ---
sound/soc/cirrus/ep93xx-i2s.c | 3 ---
sound/soc/nuc900/nuc900-ac97.c | 3 ---
sound/soc/pxa/mmp-sspa.c | 3 ---
sound/soc/tegra/tegra20_ac97.c | 7 -------
sound/soc/txx9/txx9aclc-ac97.c | 3 ---
34 files changed, 4 insertions(+), 152 deletions(-)
--
1.7.10.4
^ permalink raw reply
* Re: [PATCH v2 2/3] Input: omap-keypad: Convert to threaded IRQ
From: Felipe Balbi @ 2013-07-23 17:32 UTC (permalink / raw)
To: Felipe Balbi
Cc: Illia Smyrnov, Dmitry Torokhov, linux-input, linux-kernel,
linux-omap
In-Reply-To: <20130723172501.GC9166@radagast>
[-- Attachment #1: Type: text/plain, Size: 1923 bytes --]
Hi,
On Tue, Jul 23, 2013 at 08:25:01PM +0300, Felipe Balbi wrote:
> > Convert to use threaded IRQ.
> >
> > Cc: Felipe Balbi <balbi@ti.com>
> > Signed-off-by: Illia Smyrnov <illia.smyrnov@ti.com>
> > ---
> > drivers/input/keyboard/omap4-keypad.c | 29 ++++++++++++++++++++---------
> > 1 files changed, 20 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
> > index c727548..b876a0d 100644
> > --- a/drivers/input/keyboard/omap4-keypad.c
> > +++ b/drivers/input/keyboard/omap4-keypad.c
> > @@ -112,8 +112,22 @@ static void kbd_write_irqreg(struct omap4_keypad *keypad_data,
> > }
> >
> >
> > -/* Interrupt handler */
> > -static irqreturn_t omap4_keypad_interrupt(int irq, void *dev_id)
> > +/* Interrupt handlers */
> > +static irqreturn_t omap4_keypad_irq_handler(int irq, void *dev_id)
> > +{
> > + struct omap4_keypad *keypad_data = dev_id;
> > +
> > + if (kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)) {
> > + /* Disable interrupts */
> > + kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
> > + OMAP4_VAL_IRQDISABLE);
> > + return IRQ_WAKE_THREAD;
> > + }
> > +
> > + return IRQ_NONE;
> > +}
> > +
> > +static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id)
> > {
> > struct omap4_keypad *keypad_data = dev_id;
> > struct input_dev *input_dev = keypad_data->input;
> > @@ -121,10 +135,6 @@ static irqreturn_t omap4_keypad_interrupt(int irq, void *dev_id)
> > unsigned int col, row, code, changed;
> > u32 *new_state = (u32 *) key_state;
> >
> > - /* Disable interrupts */
> > - kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
> > - OMAP4_VAL_IRQDISABLE);
>
> looking a lot better, but I wonder if you should add a mutex to this
> threaded handler, but I guess there's no way this will never race since
s/never/ever
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ 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