Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v5] Input: add regulator haptic driver
From: Hyunhee Kim @ 2013-10-28  8:31 UTC (permalink / raw)
  To: 'Mark Brown', dmitry.torokhov, peter.ujfalusi, wfp5p,
	linux-input, linux-kernel, akpm, kyungmin.park
In-Reply-To: <20131025082944.GK18506@sirena.org.uk>

From: Hyunhee Kim <hyunhee.kim@samsung.com>
Date: Wed, 9 Oct 2013 16:21:36 +0900
Subject: [PATCH] Input: add regulator haptic driver

Signed-off-by: Hyunhee Kim <hyunhee.kim@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Aristeu Rozanski <aris@ruivo.org>
---
 drivers/input/misc/Kconfig            |   10 ++
 drivers/input/misc/Makefile           |    1 +
 drivers/input/misc/regulator-haptic.c |  187 +++++++++++++++++++++++++++++++++
 3 files changed, 198 insertions(+)
 create mode 100644 drivers/input/misc/regulator-haptic.c

diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index bb698e1..21b4d5b 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -82,6 +82,16 @@ config INPUT_ARIZONA_HAPTICS
 	  To compile this driver as a module, choose M here: the
 	  module will be called arizona-haptics.

+config INPUT_REGULATOR_HAPTIC
+	tristate "support haptics on/off using regulator"
+	select INPUT_FF_MEMLESS
+	help
+	  Say Y to enable support for the haptic module. Haptic can be
+	  enabled/disabled by regulator.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called regulator-haptic.
+
 config INPUT_BMA150
 	tristate "BMA150/SMB380 acceleration sensor support"
 	depends on I2C
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index d7fc17f..106f0bc 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_INPUT_ADXL34X_I2C)		+= adxl34x-i2c.o
 obj-$(CONFIG_INPUT_ADXL34X_SPI)		+= adxl34x-spi.o
 obj-$(CONFIG_INPUT_APANEL)		+= apanel.o
 obj-$(CONFIG_INPUT_ARIZONA_HAPTICS)	+= arizona-haptics.o
+obj-$(CONFIG_INPUT_REGULATOR_HAPTIC)	+= regulator-haptic.o
 obj-$(CONFIG_INPUT_ATI_REMOTE2)		+= ati_remote2.o
 obj-$(CONFIG_INPUT_ATLAS_BTNS)		+= atlas_btns.o
 obj-$(CONFIG_INPUT_BFIN_ROTARY)		+= bfin_rotary.o
diff --git a/drivers/input/misc/regulator-haptic.c b/drivers/input/misc/regulator-haptic.c
new file mode 100644
index 0000000..bab2ad9
--- /dev/null
+++ b/drivers/input/misc/regulator-haptic.c
@@ -0,0 +1,187 @@
+/*
+ * Regulator haptic driver
+ *
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ *
+ * Author: Hyunhee Kim <hyunhee.kim@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include <linux/regulator/driver.h>
+
+struct regulator_haptic {
+	struct device *dev;
+	struct input_dev *input_dev;
+	struct work_struct work;
+	bool enabled;
+	struct regulator *regulator;
+	struct mutex mutex;
+	int level;
+};
+
+static void regulator_haptic_enable(struct regulator_haptic *haptic)
+{
+	int ret;
+
+	if (!haptic->enabled) {
+		ret = regulator_enable(haptic->regulator);
+		if (ret)
+			dev_err(haptic->dev, "failed to enable regulator\n");
+		haptic->enabled = true;
+	}
+}
+
+static void regulator_haptic_disable(struct regulator_haptic *haptic)
+{
+	int ret;
+
+	if (haptic->enabled) {
+		ret = regulator_disable(haptic->regulator);
+		if (ret)
+			dev_err(haptic->dev, "failed to disable regulator\n");
+		haptic->enabled = false;
+	}
+}
+
+static void regulator_haptic_work(struct work_struct *work)
+{
+	struct regulator_haptic *haptic = container_of(work,
+						       struct regulator_haptic,
+						       work);
+	mutex_lock(&haptic->mutex);
+	if (haptic->level)
+		regulator_haptic_enable(haptic);
+	else
+		regulator_haptic_disable(haptic);
+	mutex_unlock(&haptic->mutex);
+}
+
+static int regulator_haptic_play(struct input_dev *input, void *data,
+				struct ff_effect *effect)
+{
+	struct regulator_haptic *haptic = input_get_drvdata(input);
+
+	haptic->level = effect->u.rumble.strong_magnitude;
+	if (!haptic->level)
+		haptic->level = effect->u.rumble.weak_magnitude;
+	schedule_work(&haptic->work);
+
+	return 0;
+}
+
+static void regulator_haptic_close(struct input_dev *input)
+{
+	struct regulator_haptic *haptic = input_get_drvdata(input);
+
+	cancel_work_sync(&haptic->work);
+	regulator_haptic_disable(haptic);
+}
+
+static int regulator_haptic_probe(struct platform_device *pdev)
+{
+	struct regulator_haptic *haptic;
+	struct input_dev *input_dev;
+	int error;
+
+	haptic = devm_kzalloc(&pdev->dev, sizeof(*haptic), GFP_KERNEL);
+	if (!haptic) {
+		dev_err(&pdev->dev, "unable to allocate memory for haptic\n");
+		return -ENOMEM;
+	}
+
+	input_dev = input_allocate_device();
+
+	if (!input_dev) {
+		dev_err(&pdev->dev, "unable to allocate memory\n");
+		return  -ENOMEM;
+	}
+
+	INIT_WORK(&haptic->work, regulator_haptic_work);
+	mutex_init(&haptic->mutex);
+	haptic->input_dev = input_dev;
+	haptic->dev = &pdev->dev;
+	haptic->regulator = regulator_get(&pdev->dev, "haptic");
+
+	if (IS_ERR(haptic->regulator)) {
+		error = PTR_ERR(haptic->regulator);
+		dev_err(&pdev->dev, "unable to get regulator, err: %d\n",
+			error);
+		goto err_ifree_mem;
+	}
+
+	haptic->input_dev->name = "regulator:haptic";
+	haptic->input_dev->dev.parent = &pdev->dev;
+	haptic->input_dev->close = regulator_haptic_close;
+	haptic->enabled = false;
+	input_set_drvdata(haptic->input_dev, haptic);
+	input_set_capability(haptic->input_dev, EV_FF, FF_RUMBLE);
+
+	error = input_ff_create_memless(input_dev, NULL,
+				      regulator_haptic_play);
+	if (error) {
+		dev_err(&pdev->dev,
+			"input_ff_create_memless() failed: %d\n",
+			error);
+		goto err_put_regulator;
+	}
+
+	error = input_register_device(haptic->input_dev);
+	if (error) {
+		dev_err(&pdev->dev,
+			"couldn't register input device: %d\n",
+			error);
+		goto err_destroy_ff;
+	}
+
+	platform_set_drvdata(pdev, haptic);
+
+	return 0;
+
+err_destroy_ff:
+	input_ff_destroy(haptic->input_dev);
+err_put_regulator:
+	regulator_put(haptic->regulator);
+err_ifree_mem:
+	input_free_device(haptic->input_dev);
+
+	return error;
+}
+
+static int regulator_haptic_remove(struct platform_device *pdev)
+{
+	struct regulator_haptic *haptic = platform_get_drvdata(pdev);
+
+	input_unregister_device(haptic->input_dev);
+	regulator_put(haptic->regulator);
+
+	return 0;
+}
+
+static struct of_device_id regulator_haptic_dt_match[] = {
+	{ .compatible = "linux,regulator-haptic" },
+	{},
+};
+
+static struct platform_driver regulator_haptic_driver = {
+	.driver		= {
+		.name	= "regulator-haptic",
+		.owner	= THIS_MODULE,
+		.of_match_table = regulator_haptic_dt_match,
+	},
+
+	.probe		= regulator_haptic_probe,
+	.remove		= regulator_haptic_remove,
+};
+module_platform_driver(regulator_haptic_driver);
+
+MODULE_ALIAS("platform:regulator-haptic");
+MODULE_DESCRIPTION("Regulator haptic driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Hyunhee Kim <hyunhee.kim@samsung.com>");
--
1.7.9.5




^ permalink raw reply related

* Re: [PATCHv2 1/3] Input: twl4030-keypad - add device tree support
From: Kumar Gala @ 2013-10-28  6:42 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Sebastian Reichel, linux-input, 'Benoît Cousson',
	Tony Lindgren, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Rob Landley, Russell King,
	Dmitry Torokhov, Grant Likely, devicetree, linux-doc,
	linux-kernel, linux-arm-kernel, linux-omap
In-Reply-To: <1382446042-27099-2-git-send-email-sre@debian.org>


On Oct 22, 2013, at 7:47 AM, Sebastian Reichel wrote:

> Add device tree support for twl4030 keypad driver and update the
> Documentation with twl4030 keypad device tree binding information.
> 
> Tested on Nokia N900.
> 
> Signed-off-by: Sebastian Reichel <sre@debian.org>
> ---
> .../devicetree/bindings/input/twl4030-keypad.txt   | 31 ++++++++
> drivers/input/keyboard/twl4030_keypad.c            | 91 ++++++++++++++++++----
> 2 files changed, 105 insertions(+), 17 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/input/twl4030-keypad.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/twl4030-keypad.txt b/Documentation/devicetree/bindings/input/twl4030-keypad.txt
> new file mode 100644
> index 0000000..2b4bd7a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/twl4030-keypad.txt
> @@ -0,0 +1,31 @@
> +* TWL4030's Keypad Controller device tree bindings
> +
> +TWL4030's Keypad controller is used to interface a SoC with a matrix-type
> +keypad device. The keypad controller supports multiple row and column lines.
> +A key can be placed at each intersection of a unique row and a unique column.
> +The keypad controller can sense a key-press and key-release and report the
> +event using a interrupt to the cpu.
> +
> +This binding is based on the matrix-keymap binding with the following
> +changes:

Maybe be a bit more specific and say 'based on the input/matrix-keymap.txt binding'..

> +
> + * keypad,num-rows and keypad,num-columns are required.
> +

Is linux,keymap required from matrix-keymap.txt?

> +Required SoC Specific Properties:
> +- compatible: should be one of the following
> +   - "ti,twl4030-keypad": For controllers compatible with twl4030 keypad
> +      controller.
> +- interrupt: should be one of the following
> +   - <1>: For controllers compatible with twl4030 keypad controller.
> +
> +Optional Properties specific to linux:
> +- linux,keypad-no-autorepeat: do no enable autorepeat feature.

Does it make sense to update the matrix-keymap.txt binding to add 'linux,keypad-no-autorepeat' there?

> +
> +Example:
> +	twl_keypad: keypad {
> +		compatible = "ti,twl4030-keypad";
> +		interrupts = <1>;
> +		keypad,num-rows = <8>;
> +		keypad,num-columns = <8>;
> +		linux,keypad-no-autorepeat;
> +	};

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Kumar Gala @ 2013-10-28  6:31 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Dmitry Torokhov, Grant Likely, Rob Herring, Peter Ujfalusi,
	Sachin Kamat, Florian Vaussard, linux-input, linux-kernel,
	devicetree
In-Reply-To: <20131026113138.GA22920@earth.universe>


On Oct 26, 2013, at 6:31 AM, Sebastian Reichel wrote:

> On Sat, Oct 26, 2013 at 01:37:57AM -0500, Kumar Gala wrote:
>> 
>> On Oct 25, 2013, at 5:18 PM, Sebastian Reichel wrote:
>> 
>>> On Fri, Oct 25, 2013 at 04:41:20PM -0500, Kumar Gala wrote:
>>>> On Oct 24, 2013, at 9:48 AM, Sebastian Reichel wrote:
>>>>> +- interrupt: should be one of the following
>>>>> +   - <8>: For controllers compatible with twl4030
>>>> 
>>>> Just checking, but the interrupt is always 8 for this device?
>>> 
>>> Yes. It's currently hardcoded in drivers/mfd/twl-core.c.
>> 
>> The fact that is hard coded in the driver does not imply that it
>> should be in the device tree binding.  Is there an interrupt
>> controller as part of the TWL4030?
> 
> Hardware looks like this:
> 
> &twl4030 {
>    compatible = "ti,twl4030";
> 	interrupt-controller;
> 	#interrupt-cells = <1>;
> 
>    twl_pwrbutton: pwrbutton {
> 		compatible = "ti,twl4030-pwrbutton";
> 		interrupts = <8>; /* 8th interrupt from the twl4030 */
> 	};
> };
> 
> Simplified the initialization of twl4030 stuff works
> like this for non DT boot:
> 
> twl4030_init(...) {
>    init_subdev(...);
>    init_subdev("twl4030-pwrbutton", ..., irq=8, ...);
>    init_subdev(...);
> };
> 
> -- Sebastian

ok, than other than Grant's comment about merging some of this together with the other twl4030 bindings, ack.

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply

* Re: [PATCHv2 1/3] Input: twl4030-keypad - add device tree support
From: Sebastian Reichel @ 2013-10-27 16:31 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: Pavel Machek, linux-input, 'Beno?t Cousson', Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	Rob Landley, Russell King, Dmitry Torokhov, Grant Likely,
	devicetree, linux-doc, linux-kernel, linux-arm-kernel, linux-omap
In-Reply-To: <20131027122347.GE15154@atomide.com>

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

On Sun, Oct 27, 2013 at 05:23:48AM -0700, Tony Lindgren wrote:
> * Pavel Machek <pavel@ucw.cz> [131027 04:48]:
> > > > > +#if IS_ENABLED(CONFIG_OF)
> > > > I'm probably missing something here, but why not #ifdef CONFIG_OF?
> > > 
> > > I have been told for other drivers, that IS_ENABLED() is
> > > the prefered way to check for configuration these days.
> > 
> > CONFIG_OF can not be module, using IS_ENABLED() on it is just wrong.
> 
> Good point. Looks like there's IS_BUILTIN that's for boolean options.

Using IS_ENABLED for boolean options is supposed to be supported
according to the comment above IS_BUILTIN:

/*
 * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
 * otherwise. For boolean options, this is equivalent to
 * IS_ENABLED(CONFIG_FOO).
 */
#define IS_BUILTIN(option) config_enabled(option)

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Grant Likely @ 2013-10-27 13:57 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Dmitry Torokhov, Rob Herring, Peter Ujfalusi, Sachin Kamat,
	Florian Vaussard, linux-input, linux-kernel, devicetree
In-Reply-To: <20131025234031.GC8657@earth.universe>

On Sat, 26 Oct 2013 01:40:31 +0200, Sebastian Reichel <sre@debian.org> wrote:
> On Fri, Oct 25, 2013 at 08:09:04PM +0100, Grant Likely wrote:
> > On Thu, 24 Oct 2013 16:48:44 +0200, Sebastian Reichel <sre@debian.org> wrote:
> > > Add device tree support for twl4030 power button driver.
> > 
> > The above commit text is insufficient. There are changes in the patch
> > that aren't described here and have nothing to do with device tree
> > bindings.
> 
> I will update the description in PATCHv7.
> 
> > [...]
> > > +++ b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> > 
> > Can all of the TWL or TWL4030 funciton bindings be collected into a
> > single file please? It is a single device after all. All of it should be
> > in bindings/mfd/twl-family.txt
> 
> I guess this should be done in another patch? There's also a typo in
> twl-family.txt's filename. My suggestion is to leave the patchset in
> its current state. I will create another patch, which combines all
> the twl4030 bindings descriptions into one file.

Yes, that is fine.

> 
> > [...]
> > > +- interrupt: should be one of the following
> > 
> > Spelling: s/interrupt/interrupts/
> 
> fixed.
> 
> > [...]
> > >  static struct platform_driver twl4030_pwrbutton_driver = {
> > > +	.probe		= twl4030_pwrbutton_probe,
> > >  	.remove		= __exit_p(twl4030_pwrbutton_remove),
> > 
> > Remove the __exit_p() wrapper. __exit is for module exit functions, not
> > remove hooks (I know, that's not actually this patch, but the code is
> > definitely wrong here).
> 
> On of the following patches converts the driver to devm, which
> results in complete removal of the twl4030_pwrbutton_remove
> function.

Okay.

g.

^ permalink raw reply

* Re: [PATCHv2 1/3] Input: twl4030-keypad - add device tree support
From: Tony Lindgren @ 2013-10-27 12:23 UTC (permalink / raw)
  To: Pavel Machek
  Cc: linux-input, 'Beno?t Cousson', Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
	Russell King, Dmitry Torokhov, Grant Likely, devicetree,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap
In-Reply-To: <20131027114753.GB14901@amd.pavel.ucw.cz>

* Pavel Machek <pavel@ucw.cz> [131027 04:48]:
> 
> > > > +#if IS_ENABLED(CONFIG_OF)
> > > I'm probably missing something here, but why not #ifdef CONFIG_OF?
> > 
> > I have been told for other drivers, that IS_ENABLED() is
> > the prefered way to check for configuration these days.
> 
> CONFIG_OF can not be module, using IS_ENABLED() on it is just wrong.

Good point. Looks like there's IS_BUILTIN that's for boolean options.

Regards,

Tony

^ permalink raw reply

* Re: [PATCHv2 1/3] Input: twl4030-keypad - add device tree support
From: Pavel Machek @ 2013-10-27 11:47 UTC (permalink / raw)
  To: linux-input-u79uwXL29TY76Z2rM5mHXA, 'Beno?t Cousson',
	Tony Lindgren, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Rob Landley, Russell King,
	Dmitry Torokhov, Grant Likely, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-omap-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20131027114026.GB14091-SfvFxonMDyemK9LvCR3Hrw@public.gmane.org>

Hi!

> > > + * keypad,num-rows and keypad,num-columns are required.
> > Is "keypad," prefix neccessary here?
> 
> See Documentation/devicetree/bindings/input/matrix-keymap.txt
> 
> > > +Optional Properties specific to linux:
> > > +- linux,keypad-no-autorepeat: do no enable autorepeat feature.
> > 
> > "do not autorepeat". Plus I do not see what is Linux specifc about not
> > autorepeating... Other systems will likely know about autorepeat, too.
> 
> This property has already been used like this for
> samsung-keypad, stmpe-keypad, omap-keypad and
> gpio-matrix-keypad.

Ok. But you still have a typo. "do no enable" => "do not enable".

> > > +#if IS_ENABLED(CONFIG_OF)
> > I'm probably missing something here, but why not #ifdef CONFIG_OF?
> 
> I have been told for other drivers, that IS_ENABLED() is
> the prefered way to check for configuration these days.

CONFIG_OF can not be module, using IS_ENABLED() on it is just wrong.

> > > @@ -381,7 +426,7 @@ static int twl4030_kp_probe(struct platform_device *pdev)
> > >  
> > >  	input_set_capability(input, EV_MSC, MSC_SCAN);
> > >  	/* Enable auto repeat feature of Linux input subsystem */
> > > -	if (pdata->rep)
> > > +	if (!kp->no_autorepeat)
> > >  		__set_bit(EV_REP, input->evbit);
> > >
> > 
> > Double negation is nasty to read. I believe code would be more
> > readable if you switched logic to kp->autorepeat.
> 
> I was thinking about the same when writing it, but decided
> against it, since it will just move the double negation to
> the variable initialization.

Well, the property should be linux,keypad-autorepeat in the first
place, but it is too late to change that.

Thanks,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
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: [PATCHv2 1/3] Input: twl4030-keypad - add device tree support
From: Sebastian Reichel @ 2013-10-27 11:40 UTC (permalink / raw)
  To: Pavel Machek
  Cc: linux-input, 'Beno?t Cousson', Tony Lindgren, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	Rob Landley, Russell King, Dmitry Torokhov, Grant Likely,
	devicetree, linux-doc, linux-kernel, linux-arm-kernel, linux-omap
In-Reply-To: <20131027111715.GA2437@xo-6d-61-c0.localdomain>

[-- Attachment #1: Type: text/plain, Size: 1672 bytes --]

Hi Pavel,

On Sun, Oct 27, 2013 at 12:17:15PM +0100, Pavel Machek wrote:
> > Add device tree support for twl4030 keypad driver and update the
> > Documentation with twl4030 keypad device tree binding information.
> > 
> > Tested on Nokia N900.
> 
> It looks pretty good.

Thanks.

> > + * keypad,num-rows and keypad,num-columns are required.
> Is "keypad," prefix neccessary here?

See Documentation/devicetree/bindings/input/matrix-keymap.txt

> > +Optional Properties specific to linux:
> > +- linux,keypad-no-autorepeat: do no enable autorepeat feature.
> 
> "do not autorepeat". Plus I do not see what is Linux specifc about not
> autorepeating... Other systems will likely know about autorepeat, too.

This property has already been used like this for
samsung-keypad, stmpe-keypad, omap-keypad and
gpio-matrix-keypad.

> > +#if IS_ENABLED(CONFIG_OF)
> I'm probably missing something here, but why not #ifdef CONFIG_OF?

I have been told for other drivers, that IS_ENABLED() is
the prefered way to check for configuration these days.

> > @@ -381,7 +426,7 @@ static int twl4030_kp_probe(struct platform_device *pdev)
> >  
> >  	input_set_capability(input, EV_MSC, MSC_SCAN);
> >  	/* Enable auto repeat feature of Linux input subsystem */
> > -	if (pdata->rep)
> > +	if (!kp->no_autorepeat)
> >  		__set_bit(EV_REP, input->evbit);
> >
> 
> Double negation is nasty to read. I believe code would be more
> readable if you switched logic to kp->autorepeat.

I was thinking about the same when writing it, but decided
against it, since it will just move the double negation to
the variable initialization.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCHv2 1/3] Input: twl4030-keypad - add device tree support
From: Pavel Machek @ 2013-10-27 11:17 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Mark Rutland, devicetree, Dmitry Torokhov, Russell King,
	Rob Landley, Pawel Moll, Stephen Warren, Tony Lindgren,
	Ian Campbell, linux-doc, linux-kernel, Rob Herring,
	Sebastian Reichel, 'Beno?t Cousson', linux-input,
	Grant Likely, linux-omap, linux-arm-kernel
In-Reply-To: <1382446042-27099-2-git-send-email-sre@debian.org>

Hi!

> Add device tree support for twl4030 keypad driver and update the
> Documentation with twl4030 keypad device tree binding information.
> 
> Tested on Nokia N900.

It looks pretty good.

> +++ b/Documentation/devicetree/bindings/input/twl4030-keypad.txt
> @@ -0,0 +1,31 @@
> +* TWL4030's Keypad Controller device tree bindings
> +
> +TWL4030's Keypad controller is used to interface a SoC with a matrix-type
> +keypad device. The keypad controller supports multiple row and column lines.
> +A key can be placed at each intersection of a unique row and a unique column.
> +The keypad controller can sense a key-press and key-release and report the
> +event using a interrupt to the cpu.
> +
> +This binding is based on the matrix-keymap binding with the following
> +changes:
> +
> + * keypad,num-rows and keypad,num-columns are required.

Is "keypad," prefix neccessary here?

> +Optional Properties specific to linux:
> +- linux,keypad-no-autorepeat: do no enable autorepeat feature.

"do not autorepeat". Plus I do not see what is Linux specifc about not
autorepeating... Other systems will likely know about autorepeat, too.

> @@ -324,6 +326,31 @@ static int twl4030_kp_program(struct twl4030_keypad *kp)
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_OF)

I'm probably missing something here, but why not #ifdef CONFIG_OF?

> @@ -381,7 +426,7 @@ static int twl4030_kp_probe(struct platform_device *pdev)
>  
>  	input_set_capability(input, EV_MSC, MSC_SCAN);
>  	/* Enable auto repeat feature of Linux input subsystem */
> -	if (pdata->rep)
> +	if (!kp->no_autorepeat)
>  		__set_bit(EV_REP, input->evbit);
>

Double negation is nasty to read. I believe code would be more
readable if you switched logic to kp->autorepeat.

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* [PATCH] HID: hid-sensor-hub: fix report size
From: Srinivas Pandruvada @ 2013-10-26 17:04 UTC (permalink / raw)
  To: jkosina; +Cc: linux-input, Srinivas Pandruvada

Most of the hid sensor field size is reported in report_size field
in the report descriptor. For rotation fusion sensor the quaternion
data is 16 byte field, the report size was set to 4 and report
count field is set to 4. So the total size is 16 bytes. But the current
driver has a bug and not taking account for report count field. This
causes user space to see only 4 bytes of data sent via IIO interface. 
The number of bytes in a field needs to take account of report_count
field. Need to multiply report_size and report_count to get total
number of bytes.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/hid-sensor-hub.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 88fc5ae..a184e19 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -326,7 +326,8 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 				field->logical == attr_usage_id) {
 				sensor_hub_fill_attr_info(info, i, report->id,
 					field->unit, field->unit_exponent,
-					field->report_size);
+					field->report_size *
+							field->report_count);
 				ret = 0;
 			} else {
 				for (j = 0; j < field->maxusage; ++j) {
@@ -338,7 +339,8 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 							i, report->id,
 							field->unit,
 							field->unit_exponent,
-							field->report_size);
+							field->report_size *
+							field->report_count);
 						ret = 0;
 						break;
 					}
@@ -425,9 +427,10 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
 		hid_dbg(hdev, "%d collection_index:%x hid:%x sz:%x\n",
 				i, report->field[i]->usage->collection_index,
 				report->field[i]->usage->hid,
-				report->field[i]->report_size/8);
-
-		sz = report->field[i]->report_size/8;
+				(report->field[i]->report_size *
+					report->field[i]->report_count)/8);
+		sz = (report->field[i]->report_size *
+					report->field[i]->report_count)/8;
 		if (pdata->pending.status && pdata->pending.attr_usage_id ==
 				report->field[i]->usage->hid) {
 			hid_dbg(hdev, "data was pending ...\n");
-- 
1.8.3.2


^ permalink raw reply related

* my subject
From: Mr. X @ 2013-10-26  8:37 UTC (permalink / raw)
  To: Recipients


Need a Loan, Loans from $5000 - $10,000,000 00. Get your no obligation FREE quote now! Repayments up to 54 Months. No Collateral, Money paid into your account within 24 hours after approval. For more Info contact us today  guaranteeloancompany1@gmail.com



^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Sebastian Reichel @ 2013-10-26 11:31 UTC (permalink / raw)
  To: Kumar Gala
  Cc: Dmitry Torokhov, Grant Likely, Rob Herring, Peter Ujfalusi,
	Sachin Kamat, Florian Vaussard, linux-input, linux-kernel,
	devicetree
In-Reply-To: <085234CB-BD53-4120-AE77-B06FA9314E96@codeaurora.org>

[-- Attachment #1: Type: text/plain, Size: 1173 bytes --]

On Sat, Oct 26, 2013 at 01:37:57AM -0500, Kumar Gala wrote:
> 
> On Oct 25, 2013, at 5:18 PM, Sebastian Reichel wrote:
> 
> > On Fri, Oct 25, 2013 at 04:41:20PM -0500, Kumar Gala wrote:
> >> On Oct 24, 2013, at 9:48 AM, Sebastian Reichel wrote:
> >>> +- interrupt: should be one of the following
> >>> +   - <8>: For controllers compatible with twl4030
> >> 
> >> Just checking, but the interrupt is always 8 for this device?
> > 
> > Yes. It's currently hardcoded in drivers/mfd/twl-core.c.
> 
> The fact that is hard coded in the driver does not imply that it
> should be in the device tree binding.  Is there an interrupt
> controller as part of the TWL4030?

Hardware looks like this:

&twl4030 {
    compatible = "ti,twl4030";
	interrupt-controller;
	#interrupt-cells = <1>;

    twl_pwrbutton: pwrbutton {
		compatible = "ti,twl4030-pwrbutton";
		interrupts = <8>; /* 8th interrupt from the twl4030 */
	};
};

Simplified the initialization of twl4030 stuff works
like this for non DT boot:

twl4030_init(...) {
    init_subdev(...);
    init_subdev("twl4030-pwrbutton", ..., irq=8, ...);
    init_subdev(...);
};

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* my subject
From: Mr. X @ 2013-10-26  8:37 UTC (permalink / raw)
  To: Recipients


Need a Loan, Loans from $5000 - $10,000,000 00. Get your no obligation FREE quote now! Repayments up to 54 Months. No Collateral, Money paid into your account within 24 hours after approval. For more Info contact us today  guaranteeloancompany1@gmail.com



^ permalink raw reply

* Re: [PATCH] Input: Adding support for touchpad on Dell XT2 model
From: Niels de Vos @ 2013-10-26  7:18 UTC (permalink / raw)
  To: Yunkang Tang
  Cc: dmitry.torokhov, cernekee, dturvene, linux-input, gaspard, jclift,
	yunkang.tang
In-Reply-To: <1382715581-2584-1-git-send-email-yunkang.tang@cn.alps.com>

On Fri, Oct 25, 2013 at 11:39:41PM +0800, Yunkang Tang wrote:
> Hi all,
> 
> This patch adding the support for touchpad on Dell XT2 model.
> It's a dual device with device ID: 73, 00, 14, that comply with "ALPS_PROTO_V2".
> 
> 
> Signed-off-by: Yunkang Tang <yunkang.tang@cn.alps.com>

Tested-by: Gaspard Jankowiak <gaspard@oknaj.eu>
Reviewed-by: Niels de Vos <ndevos@redhat.com>
    (well, for as far a one-line review goes)

Thanks!

> ---
>  drivers/input/mouse/alps.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index ca7a26f..24b3626 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -103,6 +103,7 @@ static const struct alps_model_info alps_model_data[] = {
>  	/* Dell Latitude E5500, E6400, E6500, Precision M4400 */
>  	{ { 0x62, 0x02, 0x14 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf,
>  		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },
> +	{ { 0x73, 0x00, 0x14 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_DUALPOINT },		/* Dell XT2 */
>  	{ { 0x73, 0x02, 0x50 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },		/* Dell Vostro 1400 */
>  	{ { 0x52, 0x01, 0x14 }, 0x00, ALPS_PROTO_V2, 0xff, 0xff,
>  		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },				/* Toshiba Tecra A11-11L */
> -- 
> 1.8.1.2
> 

^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Kumar Gala @ 2013-10-26  6:37 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Dmitry Torokhov, Grant Likely, Rob Herring, Peter Ujfalusi,
	Sachin Kamat, Florian Vaussard, linux-input, linux-kernel,
	devicetree
In-Reply-To: <20131025221850.GA8657@earth.universe>


On Oct 25, 2013, at 5:18 PM, Sebastian Reichel wrote:

> On Fri, Oct 25, 2013 at 04:41:20PM -0500, Kumar Gala wrote:
>> On Oct 24, 2013, at 9:48 AM, Sebastian Reichel wrote:
>>> +- interrupt: should be one of the following
>>> +   - <8>: For controllers compatible with twl4030
>> 
>> Just checking, but the interrupt is always 8 for this device?
> 
> Yes. It's currently hardcoded in drivers/mfd/twl-core.c.

The fact that is hard coded in the driver does not imply that it should be in the device tree binding.  Is there an interrupt controller as part of the TWL4030?

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Sebastian Reichel @ 2013-10-25 23:40 UTC (permalink / raw)
  To: Grant Likely
  Cc: Dmitry Torokhov, Rob Herring, Peter Ujfalusi, Sachin Kamat,
	Florian Vaussard, linux-input, linux-kernel, devicetree
In-Reply-To: <20131025190904.3EE93C403B8@trevor.secretlab.ca>

[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]

On Fri, Oct 25, 2013 at 08:09:04PM +0100, Grant Likely wrote:
> On Thu, 24 Oct 2013 16:48:44 +0200, Sebastian Reichel <sre@debian.org> wrote:
> > Add device tree support for twl4030 power button driver.
> 
> The above commit text is insufficient. There are changes in the patch
> that aren't described here and have nothing to do with device tree
> bindings.

I will update the description in PATCHv7.

> [...]
> > +++ b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> 
> Can all of the TWL or TWL4030 funciton bindings be collected into a
> single file please? It is a single device after all. All of it should be
> in bindings/mfd/twl-family.txt

I guess this should be done in another patch? There's also a typo in
twl-family.txt's filename. My suggestion is to leave the patchset in
its current state. I will create another patch, which combines all
the twl4030 bindings descriptions into one file.

> [...]
> > +- interrupt: should be one of the following
> 
> Spelling: s/interrupt/interrupts/

fixed.

> [...]
> >  static struct platform_driver twl4030_pwrbutton_driver = {
> > +	.probe		= twl4030_pwrbutton_probe,
> >  	.remove		= __exit_p(twl4030_pwrbutton_remove),
> 
> Remove the __exit_p() wrapper. __exit is for module exit functions, not
> remove hooks (I know, that's not actually this patch, but the code is
> definitely wrong here).

On of the following patches converts the driver to devm, which
results in complete removal of the twl4030_pwrbutton_remove
function.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Grant Likely @ 2013-10-25 19:09 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Torokhov
  Cc: Rob Herring, Sebastian Reichel, Peter Ujfalusi, Sachin Kamat,
	Florian Vaussard, linux-input, linux-kernel, devicetree
In-Reply-To: <1382626126-12565-2-git-send-email-sre@debian.org>

On Thu, 24 Oct 2013 16:48:44 +0200, Sebastian Reichel <sre@debian.org> wrote:
> Add device tree support for twl4030 power button driver.

The above commit text is insufficient. There are changes in the patch
that aren't described here and have nothing to do with device tree
bindings.

> 
> Signed-off-by: Sebastian Reichel <sre@debian.org>
> ---
>  .../devicetree/bindings/input/twl4030-pwrbutton.txt | 21 +++++++++++++++++++++
>  drivers/input/misc/twl4030-pwrbutton.c              | 16 ++++++++++++----
>  2 files changed, 33 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> new file mode 100644
> index 0000000..4375646
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> @@ -0,0 +1,21 @@
> +Texas Instruments TWL family (twl4030) pwrbutton module

Can all of the TWL or TWL4030 funciton bindings be collected into a
single file please? It is a single device after all. All of it should be
in bindings/mfd/twl-family.txt

> +
> +This module is part of the TWL4030. For more details about the whole
> +chip see Documentation/devicetree/bindings/mfd/twl-familly.txt.
> +
> +This module provides a simple power button event via an Interrupt.
> +
> +Required properties:
> +- compatible: should be one of the following
> +   - "ti,twl4030-pwrbutton": For controllers compatible with twl4030
> +- interrupt: should be one of the following

Spelling: s/interrupt/interrupts/

> +   - <8>: For controllers compatible with twl4030
> +
> +Example:
> +
> +&twl {
> +	twl_pwrbutton: pwrbutton {
> +		compatible = "ti,twl4030-pwrbutton";
> +		interrupts = <8>;
> +	};
> +};
> diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> index b9a05fd..a3a0fe3 100644
> --- a/drivers/input/misc/twl4030-pwrbutton.c
> +++ b/drivers/input/misc/twl4030-pwrbutton.c
> @@ -52,7 +52,7 @@ static irqreturn_t powerbutton_irq(int irq, void *_pwr)
>  	return IRQ_HANDLED;
>  }
>  
> -static int __init twl4030_pwrbutton_probe(struct platform_device *pdev)
> +static int twl4030_pwrbutton_probe(struct platform_device *pdev)
>  {
>  	struct input_dev *pwr;
>  	int irq = platform_get_irq(pdev, 0);
> @@ -106,16 +106,24 @@ static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id twl4030_pwrbutton_dt_match_table[] = {
> +       { .compatible = "ti,twl4030-pwrbutton" },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table);
> +#endif
> +
>  static struct platform_driver twl4030_pwrbutton_driver = {
> +	.probe		= twl4030_pwrbutton_probe,
>  	.remove		= __exit_p(twl4030_pwrbutton_remove),

Remove the __exit_p() wrapper. __exit is for module exit functions, not
remove hooks (I know, that's not actually this patch, but the code is
definitely wrong here).

>  	.driver		= {
>  		.name	= "twl4030_pwrbutton",
>  		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(twl4030_pwrbutton_dt_match_table),
>  	},
>  };
> -
> -module_platform_driver_probe(twl4030_pwrbutton_driver,
> -			twl4030_pwrbutton_probe);
> +module_platform_driver(twl4030_pwrbutton_driver);
>  
>  MODULE_ALIAS("platform:twl4030_pwrbutton");
>  MODULE_DESCRIPTION("Triton2 Power Button");
> -- 
> 1.8.4.rc3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Sebastian Reichel @ 2013-10-25 22:18 UTC (permalink / raw)
  To: Kumar Gala
  Cc: Dmitry Torokhov, Grant Likely, Rob Herring, Peter Ujfalusi,
	Sachin Kamat, Florian Vaussard, linux-input, linux-kernel,
	devicetree
In-Reply-To: <FC6F0077-7E61-4F64-94E5-10E8FEF1F1E4@codeaurora.org>

[-- Attachment #1: Type: text/plain, Size: 369 bytes --]

On Fri, Oct 25, 2013 at 04:41:20PM -0500, Kumar Gala wrote:
> On Oct 24, 2013, at 9:48 AM, Sebastian Reichel wrote:
> > +- interrupt: should be one of the following
> > +   - <8>: For controllers compatible with twl4030
> 
> Just checking, but the interrupt is always 8 for this device?

Yes. It's currently hardcoded in drivers/mfd/twl-core.c.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Kumar Gala @ 2013-10-25 21:41 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Sebastian Reichel, Dmitry Torokhov, Grant Likely, Rob Herring,
	Peter Ujfalusi, Sachin Kamat, Florian Vaussard, linux-input,
	linux-kernel, devicetree
In-Reply-To: <1382626126-12565-2-git-send-email-sre@debian.org>


On Oct 24, 2013, at 9:48 AM, Sebastian Reichel wrote:

> Add device tree support for twl4030 power button driver.
> 
> Signed-off-by: Sebastian Reichel <sre@debian.org>
> ---
> .../devicetree/bindings/input/twl4030-pwrbutton.txt | 21 +++++++++++++++++++++
> drivers/input/misc/twl4030-pwrbutton.c              | 16 ++++++++++++----
> 2 files changed, 33 insertions(+), 4 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> new file mode 100644
> index 0000000..4375646
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> @@ -0,0 +1,21 @@
> +Texas Instruments TWL family (twl4030) pwrbutton module
> +
> +This module is part of the TWL4030. For more details about the whole
> +chip see Documentation/devicetree/bindings/mfd/twl-familly.txt.
> +
> +This module provides a simple power button event via an Interrupt.
> +
> +Required properties:
> +- compatible: should be one of the following
> +   - "ti,twl4030-pwrbutton": For controllers compatible with twl4030
> +- interrupt: should be one of the following
> +   - <8>: For controllers compatible with twl4030

Just checking, but the interrupt is always 8 for this device?

> +
> +Example:
> +
> +&twl {
> +	twl_pwrbutton: pwrbutton {
> +		compatible = "ti,twl4030-pwrbutton";
> +		interrupts = <8>;
> +	};
> +};

Otherwise Ack on binding:

Acked-by: Kumar Gala <galak@codeaurora.org>

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply

* Re: [PATCHv6 3/3] Input: twl4030-pwrbutton: simplify driver using devm_*
From: Aaro Koskinen @ 2013-10-25 20:05 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Sebastian Reichel, Dmitry Torokhov, Grant Likely, Rob Herring,
	Peter Ujfalusi, Sachin Kamat, Florian Vaussard,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1382626126-12565-4-git-send-email-sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>

On Thu, Oct 24, 2013 at 04:48:46PM +0200, Sebastian Reichel wrote:
> Use managed irq resource to simplify the driver.
> 
> Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>

Reviewed-by: Aaro Koskinen <aaro.koskinen-X3B1VOXEql0@public.gmane.org>

> ---
>  drivers/input/misc/twl4030-pwrbutton.c | 26 ++++----------------------
>  1 file changed, 4 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> index 48639ff..be1759c 100644
> --- a/drivers/input/misc/twl4030-pwrbutton.c
> +++ b/drivers/input/misc/twl4030-pwrbutton.c
> @@ -58,7 +58,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
>  	int irq = platform_get_irq(pdev, 0);
>  	int err;
>  
> -	pwr = input_allocate_device();
> +	pwr = devm_input_allocate_device(&pdev->dev);
>  	if (!pwr) {
>  		dev_dbg(&pdev->dev, "Can't allocate power button\n");
>  		return -ENOMEM;
> @@ -70,40 +70,23 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
>  	pwr->phys = "twl4030_pwrbutton/input0";
>  	pwr->dev.parent = &pdev->dev;
>  
> -	err = request_threaded_irq(irq, NULL, powerbutton_irq,
> +	err = devm_request_threaded_irq(&pwr->dev, irq, NULL, powerbutton_irq,
>  			IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
>  			"twl4030_pwrbutton", pwr);
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
> -		goto free_input_dev;
> +		return err;
>  	}
>  
>  	err = input_register_device(pwr);
>  	if (err) {
>  		dev_err(&pdev->dev, "Can't register power button: %d\n", err);
> -		goto free_irq;
> +		return err;
>  	}
>  
>  	platform_set_drvdata(pdev, pwr);
>  
>  	return 0;
> -
> -free_irq:
> -	free_irq(irq, pwr);
> -free_input_dev:
> -	input_free_device(pwr);
> -	return err;
> -}
> -
> -static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
> -{
> -	struct input_dev *pwr = platform_get_drvdata(pdev);
> -	int irq = platform_get_irq(pdev, 0);
> -
> -	free_irq(irq, pwr);
> -	input_unregister_device(pwr);
> -
> -	return 0;
>  }
>  
>  #if IS_ENABLED(CONFIG_OF)
> @@ -116,7 +99,6 @@ MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table);
>  
>  static struct platform_driver twl4030_pwrbutton_driver = {
>  	.probe		= twl4030_pwrbutton_probe,
> -	.remove		= __exit_p(twl4030_pwrbutton_remove),
>  	.driver		= {
>  		.name	= "twl4030_pwrbutton",
>  		.owner	= THIS_MODULE,
> -- 
> 1.8.4.rc3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
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: [PATCHv6 2/3] Input: twl4030-pwrbutton: use dev_err for errors
From: Aaro Koskinen @ 2013-10-25 20:05 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Sebastian Reichel, Dmitry Torokhov, Grant Likely, Rob Herring,
	Peter Ujfalusi, Sachin Kamat, Florian Vaussard,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1382626126-12565-3-git-send-email-sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>

Hi,

On Thu, Oct 24, 2013 at 04:48:45PM +0200, Sebastian Reichel wrote:
> Use dev_err() to output errors instead of dev_dbg().
> 
> Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>

Reviewed-by: Aaro Koskinen <aaro.koskinen-X3B1VOXEql0@public.gmane.org>

> ---
>  drivers/input/misc/twl4030-pwrbutton.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> index a3a0fe3..48639ff 100644
> --- a/drivers/input/misc/twl4030-pwrbutton.c
> +++ b/drivers/input/misc/twl4030-pwrbutton.c
> @@ -74,13 +74,13 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
>  			IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
>  			"twl4030_pwrbutton", pwr);
>  	if (err < 0) {
> -		dev_dbg(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
> +		dev_err(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
>  		goto free_input_dev;
>  	}
>  
>  	err = input_register_device(pwr);
>  	if (err) {
> -		dev_dbg(&pdev->dev, "Can't register power button: %d\n", err);
> +		dev_err(&pdev->dev, "Can't register power button: %d\n", err);
>  		goto free_irq;
>  	}
>  
> -- 
> 1.8.4.rc3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
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: [PATCHv5 1/3] Input: twl4030-pwrbutton - add device tree support
From: Grant Likely @ 2013-10-25 19:40 UTC (permalink / raw)
  To: Sebastian Reichel, Rob Herring
  Cc: Dmitry Torokhov, Rob Herring, Peter Ujfalusi, Sachin Kamat,
	linux-input, linux-kernel, devicetree
In-Reply-To: <20131023224940.GA19522@earth.universe>

On Thu, 24 Oct 2013 00:49:42 +0200, Sebastian Reichel <sre@debian.org> wrote:
> On Wed, Oct 23, 2013 at 05:24:14PM -0500, Rob Herring wrote:
> > So a twl4030 device is only a power button? DT should describe the h/w
> > not a node for a sub-function of a device.
> 
> No. TWL4030 is a companion chip for the OMAP3 processor. It provides
> miscellaneous functionality, e.g.:
> 
>  * RTC
>  * Watchdog
>  * Regulators
>  * Keypad Matrix
>  * USB
>  * Audio
>  * Vibrator
>  * GPIO
>  * ...
> 
> One part of the functionality is the power button. The patch
> assumes, that the twl4030-pwrbutton node is used as follows:
> 
> twl {
>     /* ... common stuff ... */
> 
>     pwrbutton {
>         compatible = "ti,twl4030-pwrbutton";
> 		interrupts = <8>;
>     };
> };
> 
> See also:
> * Documentation/devicetree/bindings/mfd/twl-familly.txt
> * Documentation/devicetree/bindings/watchdog/twl4030-wdt.txt
> * Documentation/devicetree/bindings/sound/omap-twl4030.txt
> * Documentation/devicetree/bindings/mfd/twl4030-power.txt
> * Documentation/devicetree/bindings/mfd/twl4030-audio.txt
> * Documentation/devicetree/bindings/gpio/gpio-twl4030.txt

Wow, that's crazy! It is all one device so put all the bindings into a
single file.

g,

^ permalink raw reply

* Re: [PATCHv7][ 1/4] Input: tsc2007: Add device tree support.
From: Grant Likely @ 2013-10-25 19:11 UTC (permalink / raw)
  Cc: Mark Rutland, devicetree, Dmitry Torokhov, Eric BXXnard,
	Pawel Moll, Stephen Warren, Ian Campbell, Rob Herring,
	Denis Carikli, Thierry Reding, Sascha Hauer, linux-input,
	Shawn Guo, linux-arm-kernel
In-Reply-To: <20131025075639.75f45fb0@ipc1.ka-ro>

[-- Attachment #1: Type: text/plain, Size: 2505 bytes --]

On Fri, 25 Oct 2013 07:56:39 +0200, Lothar Waßmann <LW@KARO-electronics.de> wrote:
> Hi,
> 
> Grant Likely wrote:
> > On Thu, 24 Oct 2013 14:42:13 +0200, Denis Carikli <denis@eukrea.com> wrote:
> > > Cc: Rob Herring <rob.herring@calxeda.com>
> > > Cc: Pawel Moll <pawel.moll@arm.com>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Cc: Stephen Warren <swarren@wwwdotorg.org>
> > > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> > > Cc: devicetree@vger.kernel.org
> > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Cc: linux-input@vger.kernel.org
> > > Cc: Sascha Hauer <kernel@pengutronix.de>
> > > Cc: linux-arm-kernel@lists.infradead.org
> > > Cc: Lothar Waßmann <LW@KARO-electronics.de>
> > > Cc: Eric Bénard <eric@eukrea.com>
> > > Signed-off-by: Denis Carikli <denis@eukrea.com>
> > > ---
> > > ChangeLog v6->v7:
> > > - One small whitespace cleanup.
> > > - The properties specific to that driver are now prefixed with "ti,".
> > > - The ti,fuzzy property has now better documentation.
> > > ---
> > >  .../bindings/input/touchscreen/tsc2007.txt         |   45 +++++
> > >  drivers/input/touchscreen/tsc2007.c                |  194 +++++++++++++++-----
> > >  2 files changed, 198 insertions(+), 41 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> > > new file mode 100644
> > > index 0000000..516b63b
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> > > @@ -0,0 +1,45 @@
> > > +* Texas Instruments tsc2007 touchscreen controller
> > > +
> > > +Required properties:
> > > +- compatible: must be "ti,tsc2007".
> > > +- reg: I2C address of the chip.
> > > +- ti,x-plate-ohms: X-plate resistance in ohms.
> > > +
> > > +Optional properties:
> > > +- gpios: the interrupt gpio the chip is connected to (trough the penirq pin)
> > > +  (see GPIO binding[2] for more details).
> > 
> > Hmmm, why is this needed? Is the line used for anything other than
> > finding the irq number? If it is just an interrupt then an interrupts
> > property is all that should be specified.
> > 
> It's also the pendown detect GPIO. It asserts an interrupt when first
> activated and is then polled in the driver until it goes inactive again.

That should definitely be in the document then. Otherwise the binding
looks okay to me.

g.



[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] Input: Adding support for touchpad on Dell XT2 model
From: Yunkang Tang @ 2013-10-25 15:39 UTC (permalink / raw)
  To: dmitry.torokhov, cernekee, dturvene
  Cc: linux-input, ndevos, gaspard, jclift, yunkang.tang

Hi all,

This patch adding the support for touchpad on Dell XT2 model.
It's a dual device with device ID: 73, 00, 14, that comply with "ALPS_PROTO_V2".


Signed-off-by: Yunkang Tang <yunkang.tang@cn.alps.com>
---
 drivers/input/mouse/alps.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index ca7a26f..24b3626 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -103,6 +103,7 @@ static const struct alps_model_info alps_model_data[] = {
 	/* Dell Latitude E5500, E6400, E6500, Precision M4400 */
 	{ { 0x62, 0x02, 0x14 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf,
 		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },
+	{ { 0x73, 0x00, 0x14 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_DUALPOINT },		/* Dell XT2 */
 	{ { 0x73, 0x02, 0x50 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },		/* Dell Vostro 1400 */
 	{ { 0x52, 0x01, 0x14 }, 0x00, ALPS_PROTO_V2, 0xff, 0xff,
 		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },				/* Toshiba Tecra A11-11L */
-- 
1.8.1.2


^ permalink raw reply related

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Sebastian Reichel @ 2013-10-25 14:44 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Dmitry Torokhov, Grant Likely, Rob Herring, Sachin Kamat,
	Florian Vaussard, linux-input, linux-kernel, devicetree
In-Reply-To: <526A680A.8090308@ti.com>

[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]

Hi Péter,

On Fri, Oct 25, 2013 at 03:46:02PM +0300, Peter Ujfalusi wrote:
> > [...]
> > +#if IS_ENABLED(CONFIG_OF)
> 
> You don't need to do this.

It's done like this in all the other drivers.

> > +static const struct of_device_id twl4030_pwrbutton_dt_match_table[] = {
> > +       { .compatible = "ti,twl4030-pwrbutton" },
> > +       {},
> > +};
> > +MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table);
> > +#endif
> > +
> >  static struct platform_driver twl4030_pwrbutton_driver = {
> > +	.probe		= twl4030_pwrbutton_probe,
> >  	.remove		= __exit_p(twl4030_pwrbutton_remove),
> >  	.driver		= {
> >  		.name	= "twl4030_pwrbutton",
> >  		.owner	= THIS_MODULE,
> > +		.of_match_table = of_match_ptr(twl4030_pwrbutton_dt_match_table),
> 
> If you try to compile this driver with config !CONFIG_OF it will not work in
> this way.

For !CONFIG_OF of_match_ptr is defined as follows (in "include/linux/of.h"):

#define of_match_ptr(_ptr)  NULL

So the preprocessor will remove the undefined symbol.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox