* Re: [PATCH v2] HID: uhid: fix leak for 64/32 UHID_CREATE
From: Ben Hutchings @ 2013-11-26 14:29 UTC (permalink / raw)
To: David Herrmann; +Cc: linux-input, Jiri Kosina, stable
In-Reply-To: <1385470698-6036-1-git-send-email-dh.herrmann@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]
On Tue, 2013-11-26 at 13:58 +0100, David Herrmann wrote:
> UHID allows short writes so user-space can omit unused fields. We
> automatically set them to 0 in the kernel. However, the 64/32 bit
> compat-handler didn't do that in the UHID_CREATE fallback. This will
> reveal random kernel heap data (of random size, even) to user-space.
>
> Reported-by: Ben Hutchings <ben@decadent.org.uk>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> Cc: stable@vger.kernel.org
Fixes: befde0226a59 ('HID: uhid: make creating devices work on 64/32 systems')
(that should make it clear which versions need the fix)
> ---
> drivers/hid/uhid.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 93b00d7..cedc6da 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -287,7 +287,7 @@ static int uhid_event_from_user(const char __user *buffer, size_t len,
> */
> struct uhid_create_req_compat *compat;
>
> - compat = kmalloc(sizeof(*compat), GFP_KERNEL);
> + compat = kzalloc(sizeof(*compat), GFP_KERNEL);
> if (!compat)
> return -ENOMEM;
>
--
Ben Hutchings
Usenet is essentially a HUGE group of people passing notes in class.
- Rachel Kadel, `A Quick Guide to Newsgroup Etiquette'
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply
* [PATCH] input: ixp4xx-beeper: don't use symbols from <mach/timex.h>
From: Uwe Kleine-König @ 2013-11-26 14:29 UTC (permalink / raw)
To: Dmitry Torokhov, Krzysztof Halasa
Cc: Linus Walleij, Arnd Bergmann, linux-input, linux-arm-kernel,
Imre Kaloz, kernel
mach/timex.h is about to be dropped so don't use symbols defined in
there. For ixp4xx there is a suitable substitute for IXP4XX_TIMER_FREQ,
i.e. a global and exported variable that holds the same value.
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
Hello,
I'd like to keep that patch in a series for the arm-soc people to pull that
get's rid of <mach/timex.h>, so please don't take this patch but only ack to
keep coordinating the series as easy as possible.
I don't know who's Ack is good enough to be counted as maintainer ack, so
please point that out in your reply, too.
Thanks
Uwe
drivers/input/misc/ixp4xx-beeper.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/misc/ixp4xx-beeper.c b/drivers/input/misc/ixp4xx-beeper.c
index 17ccba8..ed8e5e8 100644
--- a/drivers/input/misc/ixp4xx-beeper.c
+++ b/drivers/input/misc/ixp4xx-beeper.c
@@ -67,7 +67,7 @@ static int ixp4xx_spkr_event(struct input_dev *dev, unsigned int type, unsigned
}
if (value > 20 && value < 32767)
- count = (IXP4XX_TIMER_FREQ / (value * 4)) - 1;
+ count = (ixp4xx_timer_freq / (value * 4)) - 1;
ixp4xx_spkr_control(pin, count);
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v2] HID: uhid: fix leak for 64/32 UHID_CREATE
From: David Herrmann @ 2013-11-26 12:58 UTC (permalink / raw)
To: linux-input; +Cc: Ben Hutchings, Jiri Kosina, David Herrmann, stable
In-Reply-To: <1385449330.23855.46.camel@deadeye.wl.decadent.org.uk>
UHID allows short writes so user-space can omit unused fields. We
automatically set them to 0 in the kernel. However, the 64/32 bit
compat-handler didn't do that in the UHID_CREATE fallback. This will
reveal random kernel heap data (of random size, even) to user-space.
Reported-by: Ben Hutchings <ben@decadent.org.uk>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Cc: stable@vger.kernel.org
---
drivers/hid/uhid.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 93b00d7..cedc6da 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -287,7 +287,7 @@ static int uhid_event_from_user(const char __user *buffer, size_t len,
*/
struct uhid_create_req_compat *compat;
- compat = kmalloc(sizeof(*compat), GFP_KERNEL);
+ compat = kzalloc(sizeof(*compat), GFP_KERNEL);
if (!compat)
return -ENOMEM;
--
1.8.4.2
^ permalink raw reply related
* Re: [PATCH] uhid: Pad short UHID_CREATE writes from compat tasks
From: David Herrmann @ 2013-11-26 12:45 UTC (permalink / raw)
To: Ben Hutchings; +Cc: David Herrmann, open list:HID CORE LAYER, Jiri Kosina
In-Reply-To: <1385449330.23855.46.camel@deadeye.wl.decadent.org.uk>
Hi
On Tue, Nov 26, 2013 at 8:02 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
> Short event writes are normally padded with zeroes, but the compat
> fixup for UHID_CREATE didn't ensure this. This appears to allow an
> information leak.
>
> Compile-tested only.
>
> Fixes: befde0226a59 ('HID: uhid: make creating devices work on 64/32 systems')
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> Cc: stable@vger.kernel.org
> ---
> I have no familiarity with uhid so I haven't written a test for this.
> It looks like it would be possible to write a UHID_CREATE event that
> only covers fields up to rd_size, and the following data on the heap
> would be copied to the HID device metadata and be readable that way.
>
> Ben.
>
> drivers/hid/uhid.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 5bf2fb7..579a7115 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -298,6 +298,9 @@ static int uhid_event_from_user(const char __user *buffer, size_t len,
> kfree(compat);
> return -EFAULT;
> }
> + if (len < sizeof(*compat))
> + memset((char *)buffer + len, 0,
> + sizeof(*compat) - len);
This should be "compat", not "buffer".
Anyhow, nice catch! But the better fix imho is to use kzalloc() for
the "compat" object. This isn't performance-critical and we can avoid
any other off-by-one bug or future conversion errors. And besides,
it's far easier to read than this memset().
Thanks
David
> /* Shuffle the data over to proper structure */
> event->type = type;
>
> --
> Ben Hutchings
> Usenet is essentially a HUGE group of people passing notes in class.
> - Rachel Kadel, `A Quick Guide to Newsgroup Etiquette'
^ permalink raw reply
* Re: [PATCHv4 0/5] HID: sony: Dualshock3 USB and LED support
From: Jiri Kosina @ 2013-11-26 9:44 UTC (permalink / raw)
To: simon; +Cc: Sven Eckelmann, linux-input
In-Reply-To: <14946321e34bb704a90c5c95f880dd9d.squirrel@mungewell.org>
On Fri, 22 Nov 2013, simon@mungewell.org wrote:
> I'm a little confused about which kernel this should be in. I believe the
> first in this patch series is required for 3.13 (but does not seemed to be
> included (1)).
>
> Without this, the kernel will be subject to lockups if anyone actually
> uses the force feedback.
Yes, the lockup fix is queued for 3.13, the rest for 3.14. See
for-3.13/upstream-fixes and for-3.14/sony branches in hid.git.
for-3.13/upstream-fixes will be going to Linus this week.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH] uhid: Pad short UHID_CREATE writes from compat tasks
From: Ben Hutchings @ 2013-11-26 7:02 UTC (permalink / raw)
To: David Herrmann; +Cc: linux-input
[-- Attachment #1: Type: text/plain, Size: 1341 bytes --]
Short event writes are normally padded with zeroes, but the compat
fixup for UHID_CREATE didn't ensure this. This appears to allow an
information leak.
Compile-tested only.
Fixes: befde0226a59 ('HID: uhid: make creating devices work on 64/32 systems')
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
Cc: stable@vger.kernel.org
---
I have no familiarity with uhid so I haven't written a test for this.
It looks like it would be possible to write a UHID_CREATE event that
only covers fields up to rd_size, and the following data on the heap
would be copied to the HID device metadata and be readable that way.
Ben.
drivers/hid/uhid.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 5bf2fb7..579a7115 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -298,6 +298,9 @@ static int uhid_event_from_user(const char __user *buffer, size_t len,
kfree(compat);
return -EFAULT;
}
+ if (len < sizeof(*compat))
+ memset((char *)buffer + len, 0,
+ sizeof(*compat) - len);
/* Shuffle the data over to proper structure */
event->type = type;
--
Ben Hutchings
Usenet is essentially a HUGE group of people passing notes in class.
- Rachel Kadel, `A Quick Guide to Newsgroup Etiquette'
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply related
* [PATCH v2] Input: ads7846: Convert to hwmon_device_register_with_groups
From: Guenter Roeck @ 2013-11-26 4:39 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, lm-sensors, linux-kernel, Guenter Roeck
Simplify the code and create mandatory 'name' attribute by using
new hwmon API.
Also use is_visible to determine visible attributes instead of creating
several different attribute groups.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Use hwmon_device_register_with_groups instead of
devm_hwmon_device_register_with_groups to keep
device removal aligned with resource removal.
drivers/input/touchscreen/ads7846.c | 81 +++++++++++------------------------
1 file changed, 25 insertions(+), 56 deletions(-)
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index ea19536..932d4cf 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -102,7 +102,6 @@ struct ads7846 {
struct regulator *reg;
#if defined(CONFIG_HWMON) || defined(CONFIG_HWMON_MODULE)
- struct attribute_group *attr_group;
struct device *hwmon;
#endif
@@ -479,42 +478,36 @@ static inline unsigned vbatt_adjust(struct ads7846 *ts, ssize_t v)
SHOW(in0_input, vaux, vaux_adjust)
SHOW(in1_input, vbatt, vbatt_adjust)
-static struct attribute *ads7846_attributes[] = {
- &dev_attr_temp0.attr,
- &dev_attr_temp1.attr,
- &dev_attr_in0_input.attr,
- &dev_attr_in1_input.attr,
- NULL,
-};
-
-static struct attribute_group ads7846_attr_group = {
- .attrs = ads7846_attributes,
-};
+static umode_t ads7846_is_visible(struct kobject *kobj, struct attribute *attr,
+ int index)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct ads7846 *ts = dev_get_drvdata(dev);
-static struct attribute *ads7843_attributes[] = {
- &dev_attr_in0_input.attr,
- &dev_attr_in1_input.attr,
- NULL,
-};
+ if (ts->model == 7843 && index < 2) /* in0, in1 */
+ return 0;
+ if (ts->model == 7845 && index != 2) /* in0 */
+ return 0;
-static struct attribute_group ads7843_attr_group = {
- .attrs = ads7843_attributes,
-};
+ return attr->mode;
+}
-static struct attribute *ads7845_attributes[] = {
- &dev_attr_in0_input.attr,
+static struct attribute *ads7846_attributes[] = {
+ &dev_attr_temp0.attr, /* 0 */
+ &dev_attr_temp1.attr, /* 1 */
+ &dev_attr_in0_input.attr, /* 2 */
+ &dev_attr_in1_input.attr, /* 3 */
NULL,
};
-static struct attribute_group ads7845_attr_group = {
- .attrs = ads7845_attributes,
+static struct attribute_group ads7846_attr_group = {
+ .attrs = ads7846_attributes,
+ .is_visible = ads7846_is_visible,
};
+__ATTRIBUTE_GROUPS(ads7846_attr);
static int ads784x_hwmon_register(struct spi_device *spi, struct ads7846 *ts)
{
- struct device *hwmon;
- int err;
-
/* hwmon sensors need a reference voltage */
switch (ts->model) {
case 7846:
@@ -535,43 +528,19 @@ static int ads784x_hwmon_register(struct spi_device *spi, struct ads7846 *ts)
break;
}
- /* different chips have different sensor groups */
- switch (ts->model) {
- case 7846:
- ts->attr_group = &ads7846_attr_group;
- break;
- case 7845:
- ts->attr_group = &ads7845_attr_group;
- break;
- case 7843:
- ts->attr_group = &ads7843_attr_group;
- break;
- default:
- dev_dbg(&spi->dev, "ADS%d not recognized\n", ts->model);
- return 0;
- }
-
- err = sysfs_create_group(&spi->dev.kobj, ts->attr_group);
- if (err)
- return err;
-
- hwmon = hwmon_device_register(&spi->dev);
- if (IS_ERR(hwmon)) {
- sysfs_remove_group(&spi->dev.kobj, ts->attr_group);
- return PTR_ERR(hwmon);
- }
+ ts->hwmon = hwmon_device_register_with_groups(&spi->dev, spi->modalias,
+ ts, ads7846_attr_groups);
+ if (IS_ERR(ts->hwmon))
+ return PTR_ERR(ts->hwmon);
- ts->hwmon = hwmon;
return 0;
}
static void ads784x_hwmon_unregister(struct spi_device *spi,
struct ads7846 *ts)
{
- if (ts->hwmon) {
- sysfs_remove_group(&spi->dev.kobj, ts->attr_group);
+ if (ts->hwmon)
hwmon_device_unregister(ts->hwmon);
- }
}
#else
--
1.7.9.7
^ permalink raw reply related
* Re: [PATCH] Input: ads7846: Convert to devm_hwmon_device_register_with_groups
From: Guenter Roeck @ 2013-11-26 4:38 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, lm-sensors, linux-kernel
In-Reply-To: <20131126030308.GF31517@core.coreip.homeip.net>
On 11/25/2013 07:03 PM, Dmitry Torokhov wrote:
> Hi Guenter,
>
> On Sat, Nov 23, 2013 at 01:37:00PM -0800, Guenter Roeck wrote:
>> Simplify the code and create mandatory 'name' attribute by using
>> new hwmon API.
>>
>> Also use is_visible to determine visible attributes instead of creating
>> several different attribute groups.
>
> This change does not quite work without converting the rest of the
> driver to devm_ resources as it makes hwmon outlive regulator and other
> essential resources when unbinding driver from the device.
>
Ok, I'll use hwmon_device_register_with_groups instead.
Thanks,
Guenter
^ permalink raw reply
* Re: [PATCHv7 0/4] twl4030-pwrbutton DT binding
From: Dmitry Torokhov @ 2013-11-26 3:29 UTC (permalink / raw)
To: linux-input, devicetree, linux-doc, linux-kernel
In-Reply-To: <20131124163343.GA31845@earth.universe>
On Sun, Nov 24, 2013 at 05:33:44PM +0100, Sebastian Reichel wrote:
> Hi,
>
> On Fri, Nov 08, 2013 at 11:11:23PM +0100, Sebastian Reichel wrote:
> > This is the seventh iteration of DT support for the TWL4030
> > power button.
>
> Dmitry, can you add this patchset to your queue?
>
> It got no further comments in two weeks.
Yes, I just applied the series.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: ads7846: Convert to devm_hwmon_device_register_with_groups
From: Dmitry Torokhov @ 2013-11-26 3:03 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-input, lm-sensors, linux-kernel
In-Reply-To: <1385242620-21719-1-git-send-email-linux@roeck-us.net>
Hi Guenter,
On Sat, Nov 23, 2013 at 01:37:00PM -0800, Guenter Roeck wrote:
> Simplify the code and create mandatory 'name' attribute by using
> new hwmon API.
>
> Also use is_visible to determine visible attributes instead of creating
> several different attribute groups.
This change does not quite work without converting the rest of the
driver to devm_ resources as it makes hwmon outlive regulator and other
essential resources when unbinding driver from the device.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 4/4 v3] Input: wacom - add SW_TOUCH to include/uapi/linux/input.h
From: Dmitry Torokhov @ 2013-11-26 2:59 UTC (permalink / raw)
To: Ping Cheng; +Cc: linux-input, killertofu, chris, peter.hutterer, Ping Cheng
In-Reply-To: <1384545602-15549-1-git-send-email-pingc@wacom.com>
Hi Ping,
On Fri, Nov 15, 2013 at 12:00:02PM -0800, Ping Cheng wrote:
>> @@ -856,6 +856,7 @@ struct input_keymap_entry {
> #define SW_FRONT_PROXIMITY 0x0b /* set = front proximity sensor active */
> #define SW_ROTATE_LOCK 0x0c /* set = rotate locked/disabled */
> #define SW_LINEIN_INSERT 0x0d /* set = inserted */
> +#define SW_TOUCH 0x0e /* set = touch switch turned on (touch events off) */
I do not think we should be adding this as is as it seems to be very
wacom-specific. I'd rather call it something else, like SW_MUTE_DEVICE
or similar.
I also wonder if this should really be a switch: can you query it's
state? What happen if user plugs in the device, engages the switch and
then [re]loads the driver? Will the state be still signalled properly?
What about suspend/resume or hibernation?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 4/5 v3] input: tc3589x-keypad: support probing from device tree
From: Dmitry Torokhov @ 2013-11-26 2:34 UTC (permalink / raw)
To: Linus Walleij
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Mark Rutland
In-Reply-To: <1384270261-15889-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Tue, Nov 12, 2013 at 04:31:01PM +0100, Linus Walleij wrote:
> Implement device tree probing for the tc3589x keypad driver.
> This is modeled on the STMPE keypad driver and tested on the
> Ux500 TVK1281618 UIB.
>
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> ChangeLog v2->v3:
> - Use two local u32 variables to avoid weirdness in u8 casting
> of the resulting values to the pointers.
> ChangeLog v1->v2:
> - Fix rows/columns binding to read two u32's insead of two
> u8 /bits/ as noted by Mark Rutland.
> ---
> drivers/input/keyboard/tc3589x-keypad.c | 66 ++++++++++++++++++++++++++++++++-
> 1 file changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/keyboard/tc3589x-keypad.c b/drivers/input/keyboard/tc3589x-keypad.c
> index 208de7cbb7fa..7f36e7addb86 100644
> --- a/drivers/input/keyboard/tc3589x-keypad.c
> +++ b/drivers/input/keyboard/tc3589x-keypad.c
> @@ -297,6 +297,65 @@ static void tc3589x_keypad_close(struct input_dev *input)
> tc3589x_keypad_disable(keypad);
> }
>
> +#ifdef CONFIG_OF
> +static const struct tc3589x_keypad_platform_data *
> +tc3589x_keypad_of_probe(struct device *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct tc3589x_keypad_platform_data *plat;
> + u32 cols, rows;
> + u32 debounce_ms;
> + int proplen;
> +
> + if (!np)
> + return ERR_PTR(-ENODEV);
> +
> + plat = devm_kzalloc(dev, sizeof(*plat), GFP_KERNEL);
> + if (!plat)
> + return ERR_PTR(-ENOMEM);
> +
> + of_property_read_u32(np, "keypad,num-columns", &cols);
> + of_property_read_u32(np, "keypad,num-rows", &rows);
> + plat->kcol = (u8) cols;
> + plat->krow = (u8) rows;
> + if (!plat->krow || !plat->kcol ||
> + plat->krow > TC_KPD_ROWS || plat->kcol > TC_KPD_COLUMNS) {
> + dev_err(dev,
> + "keypad columns/rows not properly specified (%ux%u)\n",
> + plat->kcol, plat->krow);
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (!of_get_property(np, "linux,keymap", &proplen)) {
> + dev_err(dev, "property linux,keymap not found\n");
> + return ERR_PTR(-ENOENT);
> + }
> +
> + plat->no_autorepeat = of_property_read_bool(np, "linux,no-autorepeat");
So, have DT overlords settled on the autorepeat property?
> + plat->enable_wakeup = of_property_read_bool(np, "linux,wakeup");
> +
> + /* The custom delay format is ms/16 */
> + of_property_read_u32(np, "debounce-delay-ms", &debounce_ms);
> + if (debounce_ms)
> + plat->debounce_period = debounce_ms * 16;
> + else
> + plat->debounce_period = TC_KPD_DEBOUNCE_PERIOD;
> +
> + plat->settle_time = TC_KPD_SETTLE_TIME;
> + /* FIXME: should be property of the IRQ resource? */
> + plat->irqtype = IRQF_TRIGGER_FALLING;
> +
> + return plat;
> +}
> +#else
> +static inline const struct tc3589x_keypad_platform_data *
> +tc3589x_keypad_of_probe(struct device *dev)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +#endif
> +
> +
> static int tc3589x_keypad_probe(struct platform_device *pdev)
> {
> struct tc3589x *tc3589x = dev_get_drvdata(pdev->dev.parent);
> @@ -307,8 +366,11 @@ static int tc3589x_keypad_probe(struct platform_device *pdev)
>
> plat = tc3589x->pdata->keypad;
> if (!plat) {
> - dev_err(&pdev->dev, "invalid keypad platform data\n");
> - return -EINVAL;
> + plat = tc3589x_keypad_of_probe(&pdev->dev);
> + if (IS_ERR(plat)) {
> + dev_err(&pdev->dev, "invalid keypad platform data\n");
> + return PTR_ERR(plat);
> + }
> }
>
> irq = platform_get_irq(pdev, 0);
> --
Don't you also want to add MODULE_DEVICE_TABLE entry?
Thanks.
--
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] input: don't call input_dev_release_keys() in resume
From: Dmitry Torokhov @ 2013-11-26 2:14 UTC (permalink / raw)
To: Oskar Andero; +Cc: linux-kernel, linux-input, Julian Shandorov, Aleksej Makarov
In-Reply-To: <1385126824-20446-1-git-send-email-oskar.andero@sonymobile.com>
Hi Oskar,
On Fri, Nov 22, 2013 at 02:27:04PM +0100, Oskar Andero wrote:
> From: Aleksej Makarov <aleksej.makarov@sonymobile.com>
>
> When waking up the platform by pressing a specific key, sending a
> release on that key makes it impossible to react on the event in
> user-space. This is fixed by moving the input_reset_device() call to
> resume instead.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Reviewed-by: Radovan Lekanovic <radovan.lekanovic@sonymobile.com>
> Signed-off-by: Aleksej Makarov <aleksej.makarov@sonymobile.com>
> Signed-off-by: Oskar Andero <oskar.andero@sonymobile.com>
> ---
> drivers/input/input.c | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 846ccdd..511d490 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1676,22 +1676,13 @@ static int input_dev_suspend(struct device *dev)
> {
> struct input_dev *input_dev = to_input_dev(dev);
>
> - mutex_lock(&input_dev->mutex);
> -
> - if (input_dev->users)
> - input_dev_toggle(input_dev, false);
> -
> - mutex_unlock(&input_dev->mutex);
> + input_reset_device(input_dev);
>
> return 0;
> }
>
> static int input_dev_resume(struct device *dev)
> {
> - struct input_dev *input_dev = to_input_dev(dev);
> -
> - input_reset_device(input_dev);
We still need to restore LED state after resume. Does the patch below
work for you?
Thanks.
--
Dmitry
Input: don't call input_dev_release_keys() in resume
From: Aleksej Makarov <aleksej.makarov@sonymobile.com>
When waking up the platform by pressing a specific key, sending a
release on that key makes it impossible to react on the event in
user-space. This is fixed by moving the input_reset_device() call to
resume instead.
[dmitry.torokhov@gmail.com: make sure we still restore LED/sound state
after resume, handle hibernation properly]
Signed-off-by: Aleksej Makarov <aleksej.makarov@sonymobile.com>
Signed-off-by: Oskar Andero <oskar.andero@sonymobile.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/input.c | 76 +++++++++++++++++++++++++++++++++++++------------
1 file changed, 57 insertions(+), 19 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 846ccdd..692435a 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1653,35 +1653,36 @@ static void input_dev_toggle(struct input_dev *dev, bool activate)
*/
void input_reset_device(struct input_dev *dev)
{
- mutex_lock(&dev->mutex);
+ unsigned long flags;
- if (dev->users) {
- input_dev_toggle(dev, true);
+ mutex_lock(&dev->mutex);
+ spin_lock_irqsave(&dev->event_lock, flags);
- /*
- * Keys that have been pressed at suspend time are unlikely
- * to be still pressed when we resume.
- */
- spin_lock_irq(&dev->event_lock);
- input_dev_release_keys(dev);
- spin_unlock_irq(&dev->event_lock);
- }
+ input_dev_toggle(dev, true);
+ input_dev_release_keys(dev);
+ spin_unlock_irqrestore(&dev->event_lock, flags);
mutex_unlock(&dev->mutex);
}
EXPORT_SYMBOL(input_reset_device);
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
static int input_dev_suspend(struct device *dev)
{
struct input_dev *input_dev = to_input_dev(dev);
- mutex_lock(&input_dev->mutex);
+ spin_lock_irq(&input_dev->event_lock);
- if (input_dev->users)
- input_dev_toggle(input_dev, false);
+ /*
+ * Keys that are pressed now are unlikely to be
+ * still pressed when we resume.
+ */
+ input_dev_release_keys(input_dev);
- mutex_unlock(&input_dev->mutex);
+ /* Turn off LEDs and sounds, if any are active. */
+ input_dev_toggle(input_dev, false);
+
+ spin_unlock_irq(&input_dev->event_lock);
return 0;
}
@@ -1690,7 +1691,43 @@ static int input_dev_resume(struct device *dev)
{
struct input_dev *input_dev = to_input_dev(dev);
- input_reset_device(input_dev);
+ spin_lock_irq(&input_dev->event_lock);
+
+ /* Restore state of LEDs and sounds, if any were active. */
+ input_dev_toggle(input_dev, true);
+
+ spin_unlock_irq(&input_dev->event_lock);
+
+ return 0;
+}
+
+static int input_dev_freeze(struct device *dev)
+{
+ struct input_dev *input_dev = to_input_dev(dev);
+
+ spin_lock_irq(&input_dev->event_lock);
+
+ /*
+ * Keys that are pressed now are unlikely to be
+ * still pressed when we resume.
+ */
+ input_dev_release_keys(input_dev);
+
+ spin_unlock_irq(&input_dev->event_lock);
+
+ return 0;
+}
+
+static int input_dev_poweroff(struct device *dev)
+{
+ struct input_dev *input_dev = to_input_dev(dev);
+
+ spin_lock_irq(&input_dev->event_lock);
+
+ /* Turn off LEDs and sounds, if any are active. */
+ input_dev_toggle(input_dev, false);
+
+ spin_unlock_irq(&input_dev->event_lock);
return 0;
}
@@ -1698,7 +1735,8 @@ static int input_dev_resume(struct device *dev)
static const struct dev_pm_ops input_dev_pm_ops = {
.suspend = input_dev_suspend,
.resume = input_dev_resume,
- .poweroff = input_dev_suspend,
+ .freeze = input_dev_freeze,
+ .poweroff = input_dev_poweroff,
.restore = input_dev_resume,
};
#endif /* CONFIG_PM */
@@ -1707,7 +1745,7 @@ static struct device_type input_dev_type = {
.groups = input_dev_attr_groups,
.release = input_dev_release,
.uevent = input_dev_uevent,
-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP
.pm = &input_dev_pm_ops,
#endif
};
^ permalink raw reply related
* Re: [PATCH] Input: serio: remove unnecessary pci_set_drvdata()
From: Dmitry Torokhov @ 2013-11-26 2:11 UTC (permalink / raw)
To: Jingoo Han; +Cc: linux-input
In-Reply-To: <001b01cee983$f894c510$e9be4f30$%han@samsung.com>
On Mon, Nov 25, 2013 at 11:13:46AM +0900, Jingoo Han wrote:
> The driver core clears the driver data to NULL after device_release
> or on probe failure. Thus, it is not needed to manually clear the
> device driver data to NULL.
>
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
Applied, thank you.
> ---
> drivers/input/serio/pcips2.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/input/serio/pcips2.c b/drivers/input/serio/pcips2.c
> index 76f8383..13062f6 100644
> --- a/drivers/input/serio/pcips2.c
> +++ b/drivers/input/serio/pcips2.c
> @@ -181,7 +181,6 @@ static void pcips2_remove(struct pci_dev *dev)
> struct pcips2_data *ps2if = pci_get_drvdata(dev);
>
> serio_unregister_port(ps2if->io);
> - pci_set_drvdata(dev, NULL);
> kfree(ps2if);
> pci_release_regions(dev);
> pci_disable_device(dev);
> --
> 1.7.10.4
>
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: disable interrupt on suspend
From: Jiri Kosina @ 2013-11-25 22:18 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Mika Westerberg, Jean Delvare, linux-input, linux-kernel,
Jerome Blin
In-Reply-To: <52936FC5.3060905@redhat.com>
On Mon, 25 Nov 2013, Benjamin Tissoires wrote:
> > When an I2C HID device is powered of during system sleep, as a result of
> > removing its power resources (by the ACPI core) the interrupt line might go
> > low as well. This results inadvertent interrupt and wakes the system from
> > sleep immediately.
> >
> > To prevent this we disable the device interrupt in the drivers suspend
> > method and enable it on resume. The device can still wake the system up if
> > it is wake capable (this also means that not all of its power will be
> > removed to keep the interrupt line high).
> >
> > Reported-by: Jerome Blin <jerome.blin@intel.com>
> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > ---
>
> Jiri, just FYI, I still do not have any final consumer ready i2c-hid
> hardware. So I can not test/debug anything related to the suspend-resume.
>
> If Mika tested it properly (which I expect), you may consider pulling
> this one.
Understood, thanks guys.
Applied now.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: disable interrupt on suspend
From: Benjamin Tissoires @ 2013-11-25 15:41 UTC (permalink / raw)
To: Mika Westerberg, Jiri Kosina
Cc: Jean Delvare, linux-input, linux-kernel, Jerome Blin
In-Reply-To: <1384342458-5551-1-git-send-email-mika.westerberg@linux.intel.com>
On 13/11/13 06:34, Mika Westerberg wrote:
> When an I2C HID device is powered of during system sleep, as a result of
> removing its power resources (by the ACPI core) the interrupt line might go
> low as well. This results inadvertent interrupt and wakes the system from
> sleep immediately.
>
> To prevent this we disable the device interrupt in the drivers suspend
> method and enable it on resume. The device can still wake the system up if
> it is wake capable (this also means that not all of its power will be
> removed to keep the interrupt line high).
>
> Reported-by: Jerome Blin <jerome.blin@intel.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> ---
Jiri, just FYI, I still do not have any final consumer ready i2c-hid
hardware. So I can not test/debug anything related to the suspend-resume.
If Mika tested it properly (which I expect), you may consider pulling
this one.
Cheers,
Benjamin
^ permalink raw reply
* [PATCH v2] input: Add new driver for GPIO beeper
From: Alexander Shiyan @ 2013-11-25 10:03 UTC (permalink / raw)
To: linux-input-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Dmitry Torokhov, Rob Herring,
Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
Grant Likely, Alexander Shiyan
This patch adds a new driver for the beeper controlled via GPIO pin.
The driver does not depend on the architecture and is positioned as
a replacement for the specific drivers that are used for this function.
Signed-off-by: Alexander Shiyan <shc_work-JGs/UdohzUI@public.gmane.org>
---
.../devicetree/bindings/input/gpio-beeper.txt | 13 +++
drivers/input/misc/Kconfig | 9 ++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/gpio-beeper.c | 127 +++++++++++++++++++++
4 files changed, 150 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/gpio-beeper.txt
create mode 100644 drivers/input/misc/gpio-beeper.c
diff --git a/Documentation/devicetree/bindings/input/gpio-beeper.txt b/Documentation/devicetree/bindings/input/gpio-beeper.txt
new file mode 100644
index 0000000..a5086e3
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/gpio-beeper.txt
@@ -0,0 +1,13 @@
+* GPIO beeper device tree bindings
+
+Register a beeper connected to GPIO pin.
+
+Required properties:
+- compatible: Should be "gpio-beeper".
+- gpios: From common gpio binding; gpio connection to beeper enable pin.
+
+Example:
+ beeper: beeper {
+ compatible = "gpio-beeper";
+ gpios = <&gpio3 23 GPIO_ACTIVE_HIGH>;
+ };
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 5f4967d..4ffc397 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -222,6 +222,15 @@ config INPUT_GP2A
To compile this driver as a module, choose M here: the
module will be called gp2ap002a00f.
+config INPUT_GPIO_BEEPER
+ tristate "Generic GPIO Beeper support"
+ depends on OF_GPIO
+ help
+ Say Y here if you have a beeper connected to a GPIO pin.
+
+ To compile this driver as a module, choose M here: the
+ module will be called gpio-beeper.
+
config INPUT_GPIO_TILT_POLLED
tristate "Polled GPIO tilt switch"
depends on GPIOLIB
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 0ebfb6d..cda71fc 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_INPUT_DA9052_ONKEY) += da9052_onkey.o
obj-$(CONFIG_INPUT_DA9055_ONKEY) += da9055_onkey.o
obj-$(CONFIG_INPUT_DM355EVM) += dm355evm_keys.o
obj-$(CONFIG_INPUT_GP2A) += gp2ap002a00f.o
+obj-$(CONFIG_INPUT_GPIO_BEEPER) += gpio-beeper.o
obj-$(CONFIG_INPUT_GPIO_TILT_POLLED) += gpio_tilt_polled.o
obj-$(CONFIG_HP_SDC_RTC) += hp_sdc_rtc.o
obj-$(CONFIG_INPUT_IMS_PCU) += ims-pcu.o
diff --git a/drivers/input/misc/gpio-beeper.c b/drivers/input/misc/gpio-beeper.c
new file mode 100644
index 0000000..cc81e6e
--- /dev/null
+++ b/drivers/input/misc/gpio-beeper.c
@@ -0,0 +1,127 @@
+/*
+ * Generic GPIO beeper driver
+ *
+ * Copyright (C) 2013 Alexander Shiyan <shc_work-JGs/UdohzUI@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/workqueue.h>
+#include <linux/platform_device.h>
+
+#define BEEPER_MODNAME "gpio-beeper"
+
+struct gpio_beeper {
+ struct work_struct work;
+ int gpio;
+ bool active_low;
+ bool beeping;
+};
+
+static void gpio_beeper_toggle(struct gpio_beeper *beep, bool on)
+{
+ gpio_set_value_cansleep(beep->gpio, on ^ beep->active_low);
+}
+
+static void gpio_beeper_work(struct work_struct *work)
+{
+ struct gpio_beeper *beep = container_of(work, struct gpio_beeper, work);
+
+ gpio_beeper_toggle(beep, beep->beeping);
+}
+
+static int gpio_beeper_event(struct input_dev *dev, unsigned int type,
+ unsigned int code, int value)
+{
+ struct gpio_beeper *beep = input_get_drvdata(dev);
+
+ if (type != EV_SND || code != SND_BELL)
+ return -ENOTSUPP;
+
+ if (value < 0)
+ return -EINVAL;
+
+ beep->beeping = value;
+ /* Schedule work to actually turn the beeper on or off */
+ schedule_work(&beep->work);
+
+ return 0;
+}
+
+static void gpio_beeper_close(struct input_dev *input)
+{
+ struct gpio_beeper *beep = input_get_drvdata(input);
+
+ cancel_work_sync(&beep->work);
+ gpio_beeper_toggle(beep, false);
+}
+
+static int gpio_beeper_probe(struct platform_device *pdev)
+{
+ struct gpio_beeper *beep;
+ enum of_gpio_flags flags;
+ struct input_dev *input;
+ unsigned long gflags;
+ int err;
+
+ beep = devm_kzalloc(&pdev->dev, sizeof(*beep), GFP_KERNEL);
+ if (!beep)
+ return -ENOMEM;
+
+ beep->gpio = of_get_gpio_flags(pdev->dev.of_node, 0, &flags);
+ if (!gpio_is_valid(beep->gpio))
+ return beep->gpio;
+
+ input = devm_input_allocate_device(&pdev->dev);
+ if (!input)
+ return -ENOMEM;
+
+ INIT_WORK(&beep->work, gpio_beeper_work);
+
+ input->name = pdev->name;
+ input->id.bustype = BUS_HOST;
+ input->id.vendor = 0x0001;
+ input->id.product = 0x0001;
+ input->id.version = 0x0100;
+ input->close = gpio_beeper_close;
+ input->event = gpio_beeper_event;
+
+ input_set_capability(input, EV_SND, SND_BELL);
+
+ beep->active_low = flags & OF_GPIO_ACTIVE_LOW;
+ gflags = beep->active_low ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW;
+
+ err = devm_gpio_request_one(&pdev->dev, beep->gpio, gflags, pdev->name);
+ if (err)
+ return err;
+
+ input_set_drvdata(input, beep);
+
+ return input_register_device(input);
+}
+
+static struct of_device_id gpio_beeper_of_match[] = {
+ { .compatible = BEEPER_MODNAME, },
+ { }
+};
+MODULE_DEVICE_TABLE(of, gpio_beeper_of_match);
+
+static struct platform_driver gpio_beeper_platform_driver = {
+ .driver = {
+ .name = BEEPER_MODNAME,
+ .owner = THIS_MODULE,
+ .of_match_table = gpio_beeper_of_match,
+ },
+ .probe = gpio_beeper_probe,
+};
+module_platform_driver(gpio_beeper_platform_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Alexander Shiyan <shc_work-JGs/UdohzUI@public.gmane.org>");
+MODULE_DESCRIPTION("Generic GPIO beeper driver");
--
1.8.3.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] Input: tsc2007 - remove unused poll_delay from platform data
From: Denis Carikli @ 2013-11-25 8:40 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input; +Cc: Bill Pemberton, Mark Brown, linux-kernel
In-Reply-To: <20131121051319.GA6131@core.coreip.homeip.net>
On 11/21/2013 06:13 AM, Dmitry Torokhov wrote:
> The driver does not use poll_delay parameter, so let's remove it.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
I tested on top of the "tsc2007 misc fixes" patch.
Tested-by: Denis Carikli <denis@eukrea.com>
Denis.
^ permalink raw reply
* Re: [PATCH] Input: tsc2007 - convert to use devres-managed resources
From: Denis Carikli @ 2013-11-25 8:40 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input; +Cc: Bill Pemberton, Mark Brown, linux-kernel
In-Reply-To: <20131121051438.GA6180@core.coreip.homeip.net>
On 11/21/2013 06:14 AM, Dmitry Torokhov wrote:
> This simplifies error handling path and allows us get rid of
> tsc2007_remove().
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
I tested on top of the "tsc2007 misc fixes" and the "tsc2007 - remove
unused poll_delay from platform data" patches.
Tested-by: Denis Carikli <denis@eukrea.com>
Denis.
^ permalink raw reply
* Re: [PATCHv9][ 1/3] Input: tsc2007: Add device tree support.
From: Denis Carikli @ 2013-11-25 8:39 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mark Rutland, devicetree, linux-fbdev, Eric Bénard,
Pawel Moll, Stephen Warren, Ian Campbell, Rob Herring,
Tomi Valkeinen, Sascha Hauer, linux-input, Grant Likely,
Shawn Guo, Jean-Christophe Plagniol-Villard, linux-arm-kernel,
Lothar Waßmann
In-Reply-To: <20131119211102.GA14888@core.coreip.homeip.net>
On 11/19/2013 10:11 PM, Dmitry Torokhov wrote:
> Input: tsc2007 misc fixes
>
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Tested-by: Denis Carikli <denis@eukrea.com>
Denis.
^ permalink raw reply
* [PATCH] Input: serio: remove unnecessary pci_set_drvdata()
From: Jingoo Han @ 2013-11-25 2:13 UTC (permalink / raw)
To: 'Dmitry Torokhov'
Cc: 'Dmitry Torokhov', linux-input, 'Jingoo Han'
The driver core clears the driver data to NULL after device_release
or on probe failure. Thus, it is not needed to manually clear the
device driver data to NULL.
Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
drivers/input/serio/pcips2.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/input/serio/pcips2.c b/drivers/input/serio/pcips2.c
index 76f8383..13062f6 100644
--- a/drivers/input/serio/pcips2.c
+++ b/drivers/input/serio/pcips2.c
@@ -181,7 +181,6 @@ static void pcips2_remove(struct pci_dev *dev)
struct pcips2_data *ps2if = pci_get_drvdata(dev);
serio_unregister_port(ps2if->io);
- pci_set_drvdata(dev, NULL);
kfree(ps2if);
pci_release_regions(dev);
pci_disable_device(dev);
--
1.7.10.4
^ permalink raw reply related
* Re: [PATCHv3 1/2] Input: twl4030-keypad - add device tree support
From: Sebastian Reichel @ 2013-11-24 17:17 UTC (permalink / raw)
To: Pavel Machek
Cc: Dmitry Torokhov, linux-input, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
Grant Likely, devicetree, linux-doc, linux-kernel
In-Reply-To: <20131119125031.GA14587@amd.pavel.ucw.cz>
[-- Attachment #1: Type: text/plain, Size: 774 bytes --]
Hi,
> > > I'd suggest just simple "autorepeat", so that we get rid of ugly double-negation.
> > The idea was that majority of setups want autorepeat so in the absence
> > of the property autorepeat is turned on.
>
> I see... but autorepeat on by default makes sense on devices that are
> usually querty keybaords, but does not make sense on devices that are
> usually phone keypads or power buttons.
>
> So my proposal is:
>
> 1) driver decides if it makes sense to autorepeat by default or not.
>
> 2) dts says autorepeat=0 or autorepeat=1
>
> That way, we get dts that get chance to work on other OSes, get rid of
> double negations, and get right defaults when autorepeat is not
> specified.
Sounds fine to me. Any objections?
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCHv7 0/4] twl4030-pwrbutton DT binding
From: Sebastian Reichel @ 2013-11-24 16:33 UTC (permalink / raw)
To: Dmitry Torokhov, Dmitry Torokhov
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1383948687-32279-1-git-send-email-sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 256 bytes --]
Hi,
On Fri, Nov 08, 2013 at 11:11:23PM +0100, Sebastian Reichel wrote:
> This is the seventh iteration of DT support for the TWL4030
> power button.
Dmitry, can you add this patchset to your queue?
It got no further comments in two weeks.
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCH] Input: ads7846: Convert to devm_hwmon_device_register_with_groups
From: Guenter Roeck @ 2013-11-23 21:37 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, lm-sensors, linux-kernel, Guenter Roeck
Simplify the code and create mandatory 'name' attribute by using
new hwmon API.
Also use is_visible to determine visible attributes instead of creating
several different attribute groups.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
Compiled tested only.
drivers/input/touchscreen/ads7846.c | 97 +++++++++--------------------------
1 file changed, 24 insertions(+), 73 deletions(-)
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index ea19536..38acc4c 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -101,11 +101,6 @@ struct ads7846 {
struct spi_device *spi;
struct regulator *reg;
-#if defined(CONFIG_HWMON) || defined(CONFIG_HWMON_MODULE)
- struct attribute_group *attr_group;
- struct device *hwmon;
-#endif
-
u16 model;
u16 vref_mv;
u16 vref_delay_usecs;
@@ -479,41 +474,37 @@ static inline unsigned vbatt_adjust(struct ads7846 *ts, ssize_t v)
SHOW(in0_input, vaux, vaux_adjust)
SHOW(in1_input, vbatt, vbatt_adjust)
-static struct attribute *ads7846_attributes[] = {
- &dev_attr_temp0.attr,
- &dev_attr_temp1.attr,
- &dev_attr_in0_input.attr,
- &dev_attr_in1_input.attr,
- NULL,
-};
-
-static struct attribute_group ads7846_attr_group = {
- .attrs = ads7846_attributes,
-};
+static umode_t ads7846_is_visible(struct kobject *kobj, struct attribute *attr,
+ int index)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct ads7846 *ts = dev_get_drvdata(dev);
-static struct attribute *ads7843_attributes[] = {
- &dev_attr_in0_input.attr,
- &dev_attr_in1_input.attr,
- NULL,
-};
+ if (ts->model == 7843 && index < 2) /* in0, in1 */
+ return 0;
+ if (ts->model == 7845 && index != 2) /* in0 */
+ return 0;
-static struct attribute_group ads7843_attr_group = {
- .attrs = ads7843_attributes,
-};
+ return attr->mode;
+}
-static struct attribute *ads7845_attributes[] = {
- &dev_attr_in0_input.attr,
+static struct attribute *ads7846_attributes[] = {
+ &dev_attr_temp0.attr, /* 0 */
+ &dev_attr_temp1.attr, /* 1 */
+ &dev_attr_in0_input.attr, /* 2 */
+ &dev_attr_in1_input.attr, /* 3 */
NULL,
};
-static struct attribute_group ads7845_attr_group = {
- .attrs = ads7845_attributes,
+static struct attribute_group ads7846_attr_group = {
+ .attrs = ads7846_attributes,
+ .is_visible = ads7846_is_visible,
};
+__ATTRIBUTE_GROUPS(ads7846_attr);
static int ads784x_hwmon_register(struct spi_device *spi, struct ads7846 *ts)
{
struct device *hwmon;
- int err;
/* hwmon sensors need a reference voltage */
switch (ts->model) {
@@ -535,56 +526,20 @@ static int ads784x_hwmon_register(struct spi_device *spi, struct ads7846 *ts)
break;
}
- /* different chips have different sensor groups */
- switch (ts->model) {
- case 7846:
- ts->attr_group = &ads7846_attr_group;
- break;
- case 7845:
- ts->attr_group = &ads7845_attr_group;
- break;
- case 7843:
- ts->attr_group = &ads7843_attr_group;
- break;
- default:
- dev_dbg(&spi->dev, "ADS%d not recognized\n", ts->model);
- return 0;
- }
-
- err = sysfs_create_group(&spi->dev.kobj, ts->attr_group);
- if (err)
- return err;
-
- hwmon = hwmon_device_register(&spi->dev);
- if (IS_ERR(hwmon)) {
- sysfs_remove_group(&spi->dev.kobj, ts->attr_group);
+ hwmon = devm_hwmon_device_register_with_groups(&spi->dev, spi->modalias,
+ ts, ads7846_attr_groups);
+ if (IS_ERR(hwmon))
return PTR_ERR(hwmon);
- }
- ts->hwmon = hwmon;
return 0;
}
-static void ads784x_hwmon_unregister(struct spi_device *spi,
- struct ads7846 *ts)
-{
- if (ts->hwmon) {
- sysfs_remove_group(&spi->dev.kobj, ts->attr_group);
- hwmon_device_unregister(ts->hwmon);
- }
-}
-
#else
static inline int ads784x_hwmon_register(struct spi_device *spi,
struct ads7846 *ts)
{
return 0;
}
-
-static inline void ads784x_hwmon_unregister(struct spi_device *spi,
- struct ads7846 *ts)
-{
-}
#endif
static ssize_t ads7846_pen_down_show(struct device *dev,
@@ -1447,7 +1402,7 @@ static int ads7846_probe(struct spi_device *spi)
err = sysfs_create_group(&spi->dev.kobj, &ads784x_attr_group);
if (err)
- goto err_remove_hwmon;
+ goto err_free_irq;
err = input_register_device(input_dev);
if (err)
@@ -1466,8 +1421,6 @@ static int ads7846_probe(struct spi_device *spi)
err_remove_attr_group:
sysfs_remove_group(&spi->dev.kobj, &ads784x_attr_group);
- err_remove_hwmon:
- ads784x_hwmon_unregister(spi, ts);
err_free_irq:
free_irq(spi->irq, ts);
err_disable_regulator:
@@ -1500,8 +1453,6 @@ static int ads7846_remove(struct spi_device *spi)
input_unregister_device(ts->input);
- ads784x_hwmon_unregister(spi, ts);
-
regulator_disable(ts->reg);
regulator_put(ts->reg);
--
1.7.9.7
^ permalink raw reply related
* Re: [PATCH] input:keyboard: "keycode & KEY_MAX" changes some keycode value.
From: Dmitry Torokhov @ 2013-11-23 18:06 UTC (permalink / raw)
To: andrew.liu200917
Cc: michael.hennerich, device-drivers-devel, linux-input,
linux-kernel, linux-media
In-Reply-To: <1385214345-10385-1-git-send-email-andrew.liu200917@gmail.com>
Hi Andrew,
On Sat, Nov 23, 2013 at 09:45:45PM +0800, andrew.liu200917@gmail.com wrote:
> From: Andrew Liu <andrew.liu200917@gmail.com>
>
> For exmaple, keycode: KEY_OK(0x160) is changed by "and" operation with
> KEY_MAX(0x2ff) to KEY_KPENTER(96).
>
> Signed-off-by: Andrew Liu <andrew.liu200917@gmail.com>
> ---
> drivers/input/keyboard/adp5588-keys.c | 8 ++++++--
> drivers/input/keyboard/adp5589-keys.c | 8 ++++++--
> drivers/input/keyboard/bf54x-keys.c | 8 ++++++--
> drivers/input/misc/pcf8574_keypad.c | 6 +++++-
> drivers/media/pci/ttpci/av7110_ir.c | 1 +
> 5 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/input/keyboard/adp5588-keys.c b/drivers/input/keyboard/adp5588-keys.c
> index dbd2047..1ce559e 100644
> --- a/drivers/input/keyboard/adp5588-keys.c
> +++ b/drivers/input/keyboard/adp5588-keys.c
> @@ -535,8 +535,12 @@ static int adp5588_probe(struct i2c_client *client,
> if (pdata->repeat)
> __set_bit(EV_REP, input->evbit);
>
> - for (i = 0; i < input->keycodemax; i++)
> - __set_bit(kpad->keycode[i] & KEY_MAX, input->keybit);
> + for (i = 0; i < input->keycodemax; i++) {
> + if (kpad->keycode[i] > KEY_MAX)
> + kpad->keycode[i] = 0;
There is no need to reset keymap entires to 0, not setting keybit is
enough.
> + else if (kpad->keycode[i] > KEY_RESERVED)
> + __set_bit(kpad->keycode[i], input->keybit);
> + }
> __clear_bit(KEY_RESERVED, input->keybit);
>
> if (kpad->gpimapsize)
> diff --git a/drivers/input/keyboard/adp5589-keys.c b/drivers/input/keyboard/adp5589-keys.c
> index 67d12b3..22ca2a5 100644
> --- a/drivers/input/keyboard/adp5589-keys.c
> +++ b/drivers/input/keyboard/adp5589-keys.c
> @@ -991,8 +991,12 @@ static int adp5589_probe(struct i2c_client *client,
> if (pdata->repeat)
> __set_bit(EV_REP, input->evbit);
>
> - for (i = 0; i < input->keycodemax; i++)
> - __set_bit(kpad->keycode[i] & KEY_MAX, input->keybit);
> + for (i = 0; i < input->keycodemax; i++) {
> + if (kpad->keycode[i] > KEY_MAX)
> + kpad->keycode[i] = 0;
> + else if (kpad->keycode[i] > KEY_RESERVED)
> + __set_bit(kpad->keycode[i], input->keybit);
> + }
> __clear_bit(KEY_RESERVED, input->keybit);
>
> if (kpad->gpimapsize)
> diff --git a/drivers/input/keyboard/bf54x-keys.c b/drivers/input/keyboard/bf54x-keys.c
> index fc88fb4..96f54e7 100644
> --- a/drivers/input/keyboard/bf54x-keys.c
> +++ b/drivers/input/keyboard/bf54x-keys.c
> @@ -288,8 +288,12 @@ static int bfin_kpad_probe(struct platform_device *pdev)
> if (pdata->repeat)
> __set_bit(EV_REP, input->evbit);
>
> - for (i = 0; i < input->keycodemax; i++)
> - __set_bit(bf54x_kpad->keycode[i] & KEY_MAX, input->keybit);
> + for (i = 0; i < input->keycodemax; i++) {
> + if (bf54x_kpad->keycode[i] > KEY_MAX)
> + bf54x_kpad->keycode[i] = 0;
> + else if (bf54x_kpad->keycode[i] > KEY_RESERVED)
> + __set_bit(bf54x_kpad->keycode[i], input->keybit);
> + }
> __clear_bit(KEY_RESERVED, input->keybit);
>
> error = input_register_device(input);
> diff --git a/drivers/input/misc/pcf8574_keypad.c b/drivers/input/misc/pcf8574_keypad.c
> index e373929..48829f3 100644
> --- a/drivers/input/misc/pcf8574_keypad.c
> +++ b/drivers/input/misc/pcf8574_keypad.c
> @@ -114,8 +114,12 @@ static int pcf8574_kp_probe(struct i2c_client *client, const struct i2c_device_i
>
> for (i = 0; i < ARRAY_SIZE(pcf8574_kp_btncode); i++) {
> lp->btncode[i] = pcf8574_kp_btncode[i];
> - __set_bit(lp->btncode[i] & KEY_MAX, idev->keybit);
> + if (lp->btncode[i] > KEY_MAX)
> + lp->btncode[i] = 0;
> + else if (lp->btncode[i] > KEY_RESERVED)
> + __set_bit(lp->btncode[i], idev->keybit);
> }
> + __clear_bit(KEY_RESERVED, idev->keybit);
>
> sprintf(lp->name, DRV_NAME);
> sprintf(lp->phys, "kp_data/input0");
> diff --git a/drivers/media/pci/ttpci/av7110_ir.c b/drivers/media/pci/ttpci/av7110_ir.c
> index 0e763a7..7fdac45 100644
> --- a/drivers/media/pci/ttpci/av7110_ir.c
> +++ b/drivers/media/pci/ttpci/av7110_ir.c
> @@ -201,6 +201,7 @@ static void input_register_keys(struct infrared *ir)
> else if (ir->key_map[i] > KEY_RESERVED)
> set_bit(ir->key_map[i], ir->input_dev->keybit);
> }
> + __clear_bit(KEY_RESERVED, ir->input_dev->keybit);
Here we only setting bits for keycodes above KEY_RESERVED so we do not
need to clear KEY_RESERVED bit.
>
> ir->input_dev->keycode = ir->key_map;
> ir->input_dev->keycodesize = sizeof(ir->key_map[0]);
> --
> 1.7.1
>
Edited and applied, thank you.
--
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