* Re: [PATCH v2] Input: add regulator haptic driver
From: Oliver Neukum @ 2013-10-24 8:38 UTC (permalink / raw)
To: hyunhee.kim
Cc: 'Dmitry Torokhov', broonie, peter.ujfalusi, wfp5p,
linux-input, linux-kernel, akpm, kyungmin.park,
'Aristeu Sergio Rozanski Filho'
In-Reply-To: <025101ced083$35f2a270$a1d7e750$%kim@samsung.com>
On Thu, 2013-10-24 at 15:35 +0900, hyunhee.kim wrote:
Hi,
first of all your mail client mangled the patch.
> +static void regulator_haptic_toggle(struct regulator_haptic *haptic, bool
> enable)
> +{
> + int ret;
> +
> + mutex_lock(&haptic->mutex);
> + if (enable && !haptic->enabled) {
> + haptic->enabled = true;
> + ret = regulator_enable(haptic->regulator);
> + if (ret)
> + dev_err(haptic->dev, "failed to enable
> regulator\n");
> + } else if (!enable && haptic->enabled) {
> + haptic->enabled = false;
> + ret = regulator_disable(haptic->regulator);
> + if (ret)
> + dev_err(haptic->dev, "failed to disable
> regulator\n");
> + }
> + mutex_unlock(&haptic->mutex);
> +}
> +
Is there anything gained by the toggle parameter? Just code two
functions.
Regards
Oliver
^ permalink raw reply
* Re: [PATCHv4 1/3] Input: twl4030-pwrbutton - add device tree support
From: Sebastian Reichel @ 2013-10-24 8:38 UTC (permalink / raw)
To: Florian Vaussard
Cc: Dmitry Torokhov, Grant Likely, Rob Herring, Peter Ujfalusi,
Sachin Kamat, linux-input, linux-kernel, devicetree
In-Reply-To: <5268D095.3060302@epfl.ch>
[-- Attachment #1: Type: text/plain, Size: 1580 bytes --]
Hi Florian,
On Thu, Oct 24, 2013 at 09:47:33AM +0200, Florian Vaussard wrote:
> > +Required SoC Specific 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
>
> This is <8> for your particular case, but it will depend on your
> SoC, won't it? Moreover, this property will be most likely
> inherited from the root twl node, so I do not see the need to
> document it here. See:
>
> Documentation/devicetree/bindings/mfd/twl-familly.txt
No. This is an internal twl4030 interrupt. TWL4030 functions
itself as an interrupt controller.
> > +
> > +Example:
> > + twl_pwrbutton: pwrbutton {
> > + compatible = "ti,twl4030-pwrbutton";
> > + interrupts = <8>;
> > + };
>
> You are missing the root twl node here, no?
So should I document it like this?
twl4030 {
compatible = "ti,twl4030";
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
>
> Missing #include <linux/of.h> ?
It's included indirectly, but I should probably add a direct
include. Thanks.
> Best regards,
Thanks for the comments. I will sent out an updated patchset later.
-- Sebastian
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCHv6][ 1/4] Input: tsc2007: Add device tree support.
From: Denis Carikli @ 2013-10-24 8:38 UTC (permalink / raw)
To: Sascha Hauer
Cc: Shawn Guo, Eric Bénard, linux-arm-kernel, Thierry Reding,
Dmitry Torokhov, Denis Carikli, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, devicetree,
linux-input, Lothar Waßmann
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 v5->v6:
- Fixed a code style issue.
- Improved the documentation to set some gpio related properties as optional.
- The properties related to the gpios are also explained better.
---
.../bindings/input/touchscreen/tsc2007.txt | 44 +++++
drivers/input/touchscreen/tsc2007.c | 194 +++++++++++++++-----
2 files changed, 197 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..46fc677
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
@@ -0,0 +1,44 @@
+* 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).
+- interrupt-parent: the phandle for the gpio controller
+ (see interrupt binding[1]).
+- interrupts: (gpio) interrupt to which the chip is connected
+ (see interrupt binding[1]).
+- pinctrl-0: Should specify pin control groups used for the gpio
+ (see pinctrl bindings[0]).
+- pinctrl-names: Should contain only one value - "default"
+ (see pinctrl bindings[0]).
+- max-rt: maximum pressure.
+- fuzzy: specifies the fuzz value that is used to filter noise from the event
+ stream.
+- poll-period: how much time to wait(in millisecond) before reading again the
+ values from the tsc2007.
+
+[0]: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+[1]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+[2]: Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+ &i2c1 {
+ /* ... */
+ tsc2007@49 {
+ compatible = "ti,tsc2007";
+ reg = <0x49>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_tsc2007_1>;
+ interrupt-parent = <&gpio4>;
+ interrupts = <0x0 0x8>;
+ gpios = <&gpio4 0 0>;
+ ti,x-plate-ohms = <180>;
+ };
+
+ /* ... */
+ };
diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
index 0b67ba4..6b1d7a9 100644
--- a/drivers/input/touchscreen/tsc2007.c
+++ b/drivers/input/touchscreen/tsc2007.c
@@ -26,6 +26,9 @@
#include <linux/interrupt.h>
#include <linux/i2c.h>
#include <linux/i2c/tsc2007.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
#define TSC2007_MEASURE_TEMP0 (0x0 << 4)
#define TSC2007_MEASURE_AUX (0x2 << 4)
@@ -74,7 +77,10 @@ struct tsc2007 {
u16 max_rt;
unsigned long poll_delay;
unsigned long poll_period;
+ int fuzzy;
+ char of;
+ unsigned gpio;
int irq;
wait_queue_head_t wait;
@@ -84,6 +90,11 @@ struct tsc2007 {
void (*clear_penirq)(void);
};
+static int tsc2007_get_pendown_state_dt(struct tsc2007 *ts)
+{
+ return !gpio_get_value(ts->gpio);
+}
+
static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
{
s32 data;
@@ -142,6 +153,14 @@ static u32 tsc2007_calculate_pressure(struct tsc2007 *tsc, struct ts_event *tc)
return rt;
}
+static bool tsc2007_is_pen_down_valid(struct tsc2007 *ts)
+{
+ if (ts->of)
+ return gpio_is_valid(ts->gpio);
+ else
+ return ts->get_pendown_state ? true : false;
+}
+
static bool tsc2007_is_pen_down(struct tsc2007 *ts)
{
/*
@@ -158,10 +177,13 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts)
* to fall back on the pressure reading.
*/
- if (!ts->get_pendown_state)
+ if (!tsc2007_is_pen_down_valid(ts))
return true;
- return ts->get_pendown_state();
+ if (ts->of)
+ return tsc2007_get_pendown_state_dt(ts);
+ else
+ return ts->get_pendown_state();
}
static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
@@ -178,7 +200,7 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
rt = tsc2007_calculate_pressure(ts, &tc);
- if (rt == 0 && !ts->get_pendown_state) {
+ if (!rt && !tsc2007_is_pen_down_valid(ts)) {
/*
* If pressure reported is 0 and we don't have
* callback to check pendown state, we have to
@@ -228,7 +250,7 @@ static irqreturn_t tsc2007_hard_irq(int irq, void *handle)
{
struct tsc2007 *ts = handle;
- if (!ts->get_pendown_state || likely(ts->get_pendown_state()))
+ if (tsc2007_is_pen_down(ts))
return IRQ_WAKE_THREAD;
if (ts->clear_penirq)
@@ -273,34 +295,65 @@ static void tsc2007_close(struct input_dev *input_dev)
tsc2007_stop(ts);
}
-static int tsc2007_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+#ifdef CONFIG_OF
+static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
+ struct device_node *np)
{
- struct tsc2007 *ts;
- struct tsc2007_platform_data *pdata = client->dev.platform_data;
- struct input_dev *input_dev;
- int err;
-
- if (!pdata) {
- dev_err(&client->dev, "platform data is required!\n");
+ int err = 0;
+ u32 val32;
+ u64 val64;
+
+ if (!of_property_read_u32(np, "max-rt", &val32))
+ ts->max_rt = val32;
+ else
+ ts->max_rt = MAX_12BIT;
+
+ if (!of_property_read_u32(np, "fuzzy", &val32))
+ ts->fuzzy = val32;
+
+ if (!of_property_read_u64(np, "poll-period", &val64))
+ ts->poll_period = val64;
+ else
+ ts->poll_period = 1;
+
+ if (!of_property_read_u32(np, "ti,x-plate-ohms", &val32)) {
+ ts->x_plate_ohms = val32;
+ } else {
+ dev_err(&client->dev,
+ "Error: lacking ti,x-plate-ohms devicetree property. (err %d).",
+ err);
return -EINVAL;
}
- if (!i2c_check_functionality(client->adapter,
- I2C_FUNC_SMBUS_READ_WORD_DATA))
- return -EIO;
+ ts->gpio = of_get_gpio(np, 0);
+ if (!gpio_is_valid(ts->gpio))
+ dev_err(&client->dev,
+ "GPIO not found (of_get_gpio returned %d)\n",
+ ts->gpio);
- ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL);
- input_dev = input_allocate_device();
- if (!ts || !input_dev) {
- err = -ENOMEM;
- goto err_free_mem;
- }
+ /* Used to detect if it is probed trough the device tree,
+ * in order to be able to use that information in the IRQ handler.
+ */
+ ts->of = 1;
- ts->client = client;
- ts->irq = client->irq;
- ts->input = input_dev;
- init_waitqueue_head(&ts->wait);
+ return 0;
+}
+#else
+static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
+ struct device_node *np)
+{
+ return -ENODEV;
+}
+#endif
+
+static int tsc2007_probe_pdev(struct i2c_client *client, struct tsc2007 *ts,
+ struct tsc2007_platform_data *pdata,
+ const struct i2c_device_id *id)
+{
+ if (!pdata) {
+ dev_err(&client->dev, "platform data is required!\n");
+ return -EINVAL;
+ }
ts->model = pdata->model;
ts->x_plate_ohms = pdata->x_plate_ohms;
@@ -309,13 +362,57 @@ static int tsc2007_probe(struct i2c_client *client,
ts->poll_period = pdata->poll_period ? : 1;
ts->get_pendown_state = pdata->get_pendown_state;
ts->clear_penirq = pdata->clear_penirq;
+ ts->fuzzy = pdata->fuzzy;
if (pdata->x_plate_ohms == 0) {
dev_err(&client->dev, "x_plate_ohms is not set up in platform data");
- err = -EINVAL;
- goto err_free_mem;
+ return -EINVAL;
}
+ /* Used to detect if it is probed trough the device tree,
+ * in order to be able to use that information in the IRQ handler.
+ */
+ ts->of = 0;
+
+ return 0;
+}
+
+static int tsc2007_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device_node *np = client->dev.of_node;
+ struct tsc2007_platform_data *pdata = client->dev.platform_data;
+ struct tsc2007 *ts;
+ struct input_dev *input_dev;
+ int err = 0;
+
+ ts = devm_kzalloc(&client->dev, sizeof(struct tsc2007), GFP_KERNEL);
+ if (!ts)
+ return -ENOMEM;
+
+ if (np)
+ err = tsc2007_probe_dt(client, ts, np);
+ else
+ err = tsc2007_probe_pdev(client, ts, pdata, id);
+
+ if (err)
+ return err;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_READ_WORD_DATA))
+ return -EIO;
+
+ input_dev = input_allocate_device();
+ if (!input_dev) {
+ err = -ENOMEM;
+ goto err_free_input;
+ };
+
+ ts->client = client;
+ ts->irq = client->irq;
+ ts->input = input_dev;
+ init_waitqueue_head(&ts->wait);
+
snprintf(ts->phys, sizeof(ts->phys),
"%s/input0", dev_name(&client->dev));
@@ -331,19 +428,21 @@ static int tsc2007_probe(struct i2c_client *client,
input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
- input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, pdata->fuzzx, 0);
- input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, pdata->fuzzy, 0);
+ input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, ts->fuzzy, 0);
+ input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, ts->fuzzy, 0);
input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT,
- pdata->fuzzz, 0);
+ ts->fuzzy, 0);
- if (pdata->init_platform_hw)
- pdata->init_platform_hw();
+ if (!np) {
+ if (pdata->init_platform_hw)
+ pdata->init_platform_hw();
+ }
err = request_threaded_irq(ts->irq, tsc2007_hard_irq, tsc2007_soft_irq,
IRQF_ONESHOT, client->dev.driver->name, ts);
if (err < 0) {
dev_err(&client->dev, "irq %d busy?\n", ts->irq);
- goto err_free_mem;
+ goto err_free_input;
}
tsc2007_stop(ts);
@@ -358,23 +457,27 @@ static int tsc2007_probe(struct i2c_client *client,
err_free_irq:
free_irq(ts->irq, ts);
- if (pdata->exit_platform_hw)
- pdata->exit_platform_hw();
- err_free_mem:
+ if (!np) {
+ if (pdata->exit_platform_hw)
+ pdata->exit_platform_hw();
+ }
+ err_free_input:
input_free_device(input_dev);
- kfree(ts);
return err;
}
static int tsc2007_remove(struct i2c_client *client)
{
+ struct device_node *np = client->dev.of_node;
struct tsc2007 *ts = i2c_get_clientdata(client);
struct tsc2007_platform_data *pdata = client->dev.platform_data;
free_irq(ts->irq, ts);
- if (pdata->exit_platform_hw)
- pdata->exit_platform_hw();
+ if (!np) {
+ if (pdata->exit_platform_hw)
+ pdata->exit_platform_hw();
+ }
input_unregister_device(ts->input);
kfree(ts);
@@ -389,10 +492,19 @@ static const struct i2c_device_id tsc2007_idtable[] = {
MODULE_DEVICE_TABLE(i2c, tsc2007_idtable);
+#ifdef CONFIG_OF
+static const struct of_device_id tsc2007_of_match[] = {
+ { .compatible = "ti,tsc2007" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tsc2007_of_match);
+#endif
+
static struct i2c_driver tsc2007_driver = {
.driver = {
.owner = THIS_MODULE,
- .name = "tsc2007"
+ .name = "tsc2007",
+ .of_match_table = of_match_ptr(tsc2007_of_match),
},
.id_table = tsc2007_idtable,
.probe = tsc2007_probe,
--
1.7.9.5
--
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
* [PATCHv6][ 2/4] ARM: dts: cpuimx51 Add touchscreen support.
From: Denis Carikli @ 2013-10-24 8:38 UTC (permalink / raw)
To: Sascha Hauer
Cc: Shawn Guo, Eric Bénard, linux-arm-kernel, Thierry Reding,
Dmitry Torokhov, Denis Carikli, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, devicetree,
linux-input, Lothar Waßmann
In-Reply-To: <1382603933-8539-1-git-send-email-denis@eukrea.com>
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>
---
arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi b/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
index 8638656..34ca8d3a 100644
--- a/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
+++ b/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
@@ -42,6 +42,17 @@
compatible = "nxp,pcf8563";
reg = <0x51>;
};
+
+ tsc2007: tsc2007@49 {
+ compatible = "ti,tsc2007";
+ reg = <0x49>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_tsc2007_1>;
+ interrupt-parent = <&gpio4>;
+ interrupts = <0x0 0x8>;
+ gpios = <&gpio4 0 0>;
+ ti,x-plate-ohms = <180>;
+ };
};
&iomuxc {
--
1.7.9.5
--
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
* [PATCHv6][ 3/4] ARM: dts: cpuimx35 Add touchscreen support.
From: Denis Carikli @ 2013-10-24 8:38 UTC (permalink / raw)
To: Sascha Hauer
Cc: Shawn Guo, Eric Bénard, linux-arm-kernel, Thierry Reding,
Dmitry Torokhov, Denis Carikli, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, devicetree,
linux-input, Lothar Waßmann
In-Reply-To: <1382603933-8539-1-git-send-email-denis@eukrea.com>
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>
---
arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
index 2c2d4cd..a22230b 100644
--- a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
+++ b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
@@ -41,6 +41,17 @@
compatible = "nxp,pcf8563";
reg = <0x51>;
};
+
+ tsc2007: tsc2007@48 {
+ compatible = "ti,tsc2007";
+ reg = <0x48>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_tsc2007_1>;
+ interrupt-parent = <&gpio3>;
+ interrupts = <0x2 0x8>;
+ gpios = <&gpio3 2 0>;
+ ti,x-plate-ohms = <180>;
+ };
};
&iomuxc {
--
1.7.9.5
--
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
* [PATCHv6][ 4/4] ARM: imx_v6_v7_defconfig: Enable tsc2007 support.
From: Denis Carikli @ 2013-10-24 8:38 UTC (permalink / raw)
To: Sascha Hauer
Cc: Shawn Guo, Eric Bénard, linux-arm-kernel, Thierry Reding,
Dmitry Torokhov, Denis Carikli, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, devicetree,
linux-input, Lothar Waßmann, Fabio Estevam
In-Reply-To: <1382603933-8539-1-git-send-email-denis@eukrea.com>
The eukrea cpuimx35 and cpuimx51 have a tsc2007 touchscreen controller,
so we turn it on.
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: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
arch/arm/configs/imx_v6_v7_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index d8a52a0..23ba17d 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -130,6 +130,7 @@ CONFIG_MOUSE_PS2_ELANTECH=y
CONFIG_INPUT_TOUCHSCREEN=y
CONFIG_TOUCHSCREEN_EGALAX=y
CONFIG_TOUCHSCREEN_MC13783=y
+CONFIG_TOUCHSCREEN_TSC2007=y
CONFIG_INPUT_MISC=y
CONFIG_INPUT_MMA8450=y
CONFIG_SERIO_SERPORT=m
--
1.7.9.5
--
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
* Re: [PATCHv4 1/3] Input: twl4030-pwrbutton - add device tree support
From: Florian Vaussard @ 2013-10-24 9:16 UTC (permalink / raw)
To: Dmitry Torokhov, Grant Likely, Rob Herring, Peter Ujfalusi,
Sachin Kamat, linux-input, linux-kernel, devicetree
In-Reply-To: <20131024083837.GA28938@earth.universe>
Hello,
On 10/24/2013 10:38 AM, Sebastian Reichel wrote:
> Hi Florian,
>
> On Thu, Oct 24, 2013 at 09:47:33AM +0200, Florian Vaussard wrote:
>>> +Required SoC Specific 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
>>
>> This is <8> for your particular case, but it will depend on your
>> SoC, won't it? Moreover, this property will be most likely
>> inherited from the root twl node, so I do not see the need to
>> document it here. See:
>>
>> Documentation/devicetree/bindings/mfd/twl-familly.txt
>
> No. This is an internal twl4030 interrupt. TWL4030 functions
> itself as an interrupt controller.
>
So if it does not belong to the TWL parent, where is it used in your code?
You should be parsing this property, so you can set up the IRQ properly.
I am a bit confused here. If it is fixed, no need for a OF property.
>>> +
>>> +Example:
>>> + twl_pwrbutton: pwrbutton {
>>> + compatible = "ti,twl4030-pwrbutton";
>>> + interrupts = <8>;
>>> + };
>>
>> You are missing the root twl node here, no?
>
> So should I document it like this?
>
IMHO it is more clear for the user.
> twl4030 {
> compatible = "ti,twl4030";
>
> pwrbutton {
> compatible = "ti,twl4030-pwrbutton";
> interrupts = <8>;
> };
> };
Nit, but existing documentations follow the "name@address"
form for the root node, as the TWL is on an I2C bus.
Either it is already defined, thus you should use "&twl4030"
to reference it, or you create the TWL node and something like
"twl4030@48" should be used.
For an example, you can refer to existing bindings, like
Documentation/devicetree/bindings/mfd/twl4030-audio.txt.
Best regards,
Florian
^ permalink raw reply
* [PATCH v3] Input: add regulator haptic driver
From: hyunhee.kim @ 2013-10-24 9:19 UTC (permalink / raw)
To: 'Oliver Neukum'
Cc: 'Dmitry Torokhov', broonie, peter.ujfalusi, wfp5p,
linux-input, linux-kernel, akpm, kyungmin.park,
'Aristeu Sergio Rozanski Filho'
In-Reply-To: <1382603886.1559.11.camel@linux-fkkt.site>
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 | 184 +++++++++++++++++++++++++++++++++
3 files changed, 195 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..882c8b9
--- /dev/null
+++ b/drivers/input/misc/regulator-haptic.c
@@ -0,0 +1,184 @@
+/*
+ * 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_toggle(struct regulator_haptic *haptic, bool enable)
+{
+ int ret;
+
+ mutex_lock(&haptic->mutex);
+ if (enable && !haptic->enabled) {
+ haptic->enabled = true;
+ ret = regulator_enable(haptic->regulator);
+ if (ret)
+ dev_err(haptic->dev, "failed to enable regulator\n");
+ } else if (!enable && haptic->enabled) {
+ haptic->enabled = false;
+ ret = regulator_disable(haptic->regulator);
+ if (ret)
+ dev_err(haptic->dev, "failed to disable regulator\n");
+ }
+ mutex_unlock(&haptic->mutex);
+}
+
+static void regulator_haptic_work(struct work_struct *work)
+{
+ struct regulator_haptic *haptic = container_of(work,
+ struct regulator_haptic,
+ work);
+ if (haptic->level)
+ regulator_haptic_toggle(haptic, true);
+ else
+ regulator_haptic_toggle(haptic, false);
+
+}
+
+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_toggle(haptic, false);
+}
+
+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");
+ error = -ENOMEM;
+ goto err_kfree_mem;
+ }
+
+ 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);
+err_kfree_mem:
+ kfree(haptic);
+
+ 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: [PATCH v2] Input: add regulator haptic driver
From: hyunhee.kim @ 2013-10-24 9:26 UTC (permalink / raw)
To: 'Oliver Neukum'
Cc: 'Dmitry Torokhov', broonie, peter.ujfalusi, wfp5p,
linux-input, linux-kernel, akpm, kyungmin.park,
'Aristeu Sergio Rozanski Filho'
In-Reply-To: <1382603886.1559.11.camel@linux-fkkt.site>
Hi,
Thanks for your review.
I resent patch v3 after removing wrong wrapping.
I made one toggle function because enable/disable functions have redundant codes and another reviewer suggested it.
Is it better to separate it into two functions?
Thanks,
Hyunhee Kim.
-----Original Message-----
From: Oliver Neukum [mailto:oneukum@suse.de]
Sent: Thursday, October 24, 2013 5:38 PM
To: hyunhee.kim
Cc: 'Dmitry Torokhov'; broonie@opensource.wolfsonmicro.com; peter.ujfalusi@ti.com; wfp5p@virginia.edu; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; akpm@linux-foundation.org; kyungmin.park@samsung.com; 'Aristeu Sergio Rozanski Filho'
Subject: Re: [PATCH v2] Input: add regulator haptic driver
On Thu, 2013-10-24 at 15:35 +0900, hyunhee.kim wrote:
Hi,
first of all your mail client mangled the patch.
> +static void regulator_haptic_toggle(struct regulator_haptic *haptic, bool
> enable)
> +{
> + int ret;
> +
> + mutex_lock(&haptic->mutex);
> + if (enable && !haptic->enabled) {
> + haptic->enabled = true;
> + ret = regulator_enable(haptic->regulator);
> + if (ret)
> + dev_err(haptic->dev, "failed to enable
> regulator\n");
> + } else if (!enable && haptic->enabled) {
> + haptic->enabled = false;
> + ret = regulator_disable(haptic->regulator);
> + if (ret)
> + dev_err(haptic->dev, "failed to disable
> regulator\n");
> + }
> + mutex_unlock(&haptic->mutex);
> +}
> +
Is there anything gained by the toggle parameter? Just code two
functions.
Regards
Oliver
^ permalink raw reply
* Re: [PATCHv6][ 1/4] Input: tsc2007: Add device tree support.
From: Grant Likely @ 2013-10-24 10:16 UTC (permalink / raw)
To: Sascha Hauer
Cc: Mark Rutland, devicetree, Ian Campbell, Pawel Moll,
Stephen Warren, Dmitry Torokhov, Rob Herring, Denis Carikli,
Thierry Reding, linux-input, Shawn Guo, linux-arm-kernel
In-Reply-To: <1382603933-8539-1-git-send-email-denis@eukrea.com>
[-- Attachment #1: Type: text/plain, Size: 12505 bytes --]
On Thu, 24 Oct 2013 10:38:50 +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 v5->v6:
> - Fixed a code style issue.
> - Improved the documentation to set some gpio related properties as optional.
> - The properties related to the gpios are also explained better.
> ---
> .../bindings/input/touchscreen/tsc2007.txt | 44 +++++
> drivers/input/touchscreen/tsc2007.c | 194 +++++++++++++++-----
> 2 files changed, 197 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..46fc677
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> @@ -0,0 +1,44 @@
> +* 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).
> +- interrupt-parent: the phandle for the gpio controller
> + (see interrupt binding[1]).
> +- interrupts: (gpio) interrupt to which the chip is connected
> + (see interrupt binding[1]).
> +- pinctrl-0: Should specify pin control groups used for the gpio
> + (see pinctrl bindings[0]).
> +- pinctrl-names: Should contain only one value - "default"
> + (see pinctrl bindings[0]).
> +- max-rt: maximum pressure.
> +- fuzzy: specifies the fuzz value that is used to filter noise from the event
> + stream.
I don't actually understand what this means, can you clarify in the
document?
> +- poll-period: how much time to wait(in millisecond) before reading again the
> + values from the tsc2007.
max-rt, fuzzy, and poll-period should probably be prefixed with "ti,".
Otherwise the binding looks good to me.
g.
> +
> +[0]: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
> +[1]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
> +[2]: Documentation/devicetree/bindings/gpio/gpio.txt
> +
> +Example:
> + &i2c1 {
> + /* ... */
> + tsc2007@49 {
> + compatible = "ti,tsc2007";
> + reg = <0x49>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_tsc2007_1>;
> + interrupt-parent = <&gpio4>;
> + interrupts = <0x0 0x8>;
> + gpios = <&gpio4 0 0>;
> + ti,x-plate-ohms = <180>;
> + };
> +
> + /* ... */
> + };
> diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
> index 0b67ba4..6b1d7a9 100644
> --- a/drivers/input/touchscreen/tsc2007.c
> +++ b/drivers/input/touchscreen/tsc2007.c
> @@ -26,6 +26,9 @@
> #include <linux/interrupt.h>
> #include <linux/i2c.h>
> #include <linux/i2c/tsc2007.h>
> +#include <linux/of_device.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
>
> #define TSC2007_MEASURE_TEMP0 (0x0 << 4)
> #define TSC2007_MEASURE_AUX (0x2 << 4)
> @@ -74,7 +77,10 @@ struct tsc2007 {
> u16 max_rt;
> unsigned long poll_delay;
> unsigned long poll_period;
> + int fuzzy;
> + char of;
>
> + unsigned gpio;
> int irq;
>
> wait_queue_head_t wait;
> @@ -84,6 +90,11 @@ struct tsc2007 {
> void (*clear_penirq)(void);
> };
>
> +static int tsc2007_get_pendown_state_dt(struct tsc2007 *ts)
> +{
> + return !gpio_get_value(ts->gpio);
> +}
> +
> static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
> {
> s32 data;
> @@ -142,6 +153,14 @@ static u32 tsc2007_calculate_pressure(struct tsc2007 *tsc, struct ts_event *tc)
> return rt;
> }
>
> +static bool tsc2007_is_pen_down_valid(struct tsc2007 *ts)
> +{
> + if (ts->of)
> + return gpio_is_valid(ts->gpio);
> + else
> + return ts->get_pendown_state ? true : false;
> +}
> +
> static bool tsc2007_is_pen_down(struct tsc2007 *ts)
> {
> /*
> @@ -158,10 +177,13 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts)
> * to fall back on the pressure reading.
> */
>
> - if (!ts->get_pendown_state)
> + if (!tsc2007_is_pen_down_valid(ts))
> return true;
>
> - return ts->get_pendown_state();
> + if (ts->of)
> + return tsc2007_get_pendown_state_dt(ts);
> + else
> + return ts->get_pendown_state();
> }
>
> static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
> @@ -178,7 +200,7 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
>
> rt = tsc2007_calculate_pressure(ts, &tc);
>
> - if (rt == 0 && !ts->get_pendown_state) {
> + if (!rt && !tsc2007_is_pen_down_valid(ts)) {
> /*
> * If pressure reported is 0 and we don't have
> * callback to check pendown state, we have to
> @@ -228,7 +250,7 @@ static irqreturn_t tsc2007_hard_irq(int irq, void *handle)
> {
> struct tsc2007 *ts = handle;
>
> - if (!ts->get_pendown_state || likely(ts->get_pendown_state()))
> + if (tsc2007_is_pen_down(ts))
> return IRQ_WAKE_THREAD;
>
> if (ts->clear_penirq)
> @@ -273,34 +295,65 @@ static void tsc2007_close(struct input_dev *input_dev)
> tsc2007_stop(ts);
> }
>
> -static int tsc2007_probe(struct i2c_client *client,
> - const struct i2c_device_id *id)
> +#ifdef CONFIG_OF
> +static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
> + struct device_node *np)
> {
> - struct tsc2007 *ts;
> - struct tsc2007_platform_data *pdata = client->dev.platform_data;
> - struct input_dev *input_dev;
> - int err;
> -
> - if (!pdata) {
> - dev_err(&client->dev, "platform data is required!\n");
> + int err = 0;
> + u32 val32;
> + u64 val64;
> +
> + if (!of_property_read_u32(np, "max-rt", &val32))
> + ts->max_rt = val32;
> + else
> + ts->max_rt = MAX_12BIT;
> +
> + if (!of_property_read_u32(np, "fuzzy", &val32))
> + ts->fuzzy = val32;
> +
> + if (!of_property_read_u64(np, "poll-period", &val64))
> + ts->poll_period = val64;
> + else
> + ts->poll_period = 1;
> +
> + if (!of_property_read_u32(np, "ti,x-plate-ohms", &val32)) {
> + ts->x_plate_ohms = val32;
> + } else {
> + dev_err(&client->dev,
> + "Error: lacking ti,x-plate-ohms devicetree property. (err %d).",
> + err);
> return -EINVAL;
> }
>
> - if (!i2c_check_functionality(client->adapter,
> - I2C_FUNC_SMBUS_READ_WORD_DATA))
> - return -EIO;
> + ts->gpio = of_get_gpio(np, 0);
> + if (!gpio_is_valid(ts->gpio))
> + dev_err(&client->dev,
> + "GPIO not found (of_get_gpio returned %d)\n",
> + ts->gpio);
>
> - ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL);
> - input_dev = input_allocate_device();
> - if (!ts || !input_dev) {
> - err = -ENOMEM;
> - goto err_free_mem;
> - }
> + /* Used to detect if it is probed trough the device tree,
> + * in order to be able to use that information in the IRQ handler.
> + */
> + ts->of = 1;
>
> - ts->client = client;
> - ts->irq = client->irq;
> - ts->input = input_dev;
> - init_waitqueue_head(&ts->wait);
> + return 0;
> +}
> +#else
> +static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
> + struct device_node *np)
> +{
> + return -ENODEV;
> +}
> +#endif
> +
> +static int tsc2007_probe_pdev(struct i2c_client *client, struct tsc2007 *ts,
> + struct tsc2007_platform_data *pdata,
> + const struct i2c_device_id *id)
> +{
> + if (!pdata) {
> + dev_err(&client->dev, "platform data is required!\n");
> + return -EINVAL;
> + }
>
> ts->model = pdata->model;
> ts->x_plate_ohms = pdata->x_plate_ohms;
> @@ -309,13 +362,57 @@ static int tsc2007_probe(struct i2c_client *client,
> ts->poll_period = pdata->poll_period ? : 1;
> ts->get_pendown_state = pdata->get_pendown_state;
> ts->clear_penirq = pdata->clear_penirq;
> + ts->fuzzy = pdata->fuzzy;
>
> if (pdata->x_plate_ohms == 0) {
> dev_err(&client->dev, "x_plate_ohms is not set up in platform data");
> - err = -EINVAL;
> - goto err_free_mem;
> + return -EINVAL;
> }
>
> + /* Used to detect if it is probed trough the device tree,
> + * in order to be able to use that information in the IRQ handler.
> + */
> + ts->of = 0;
> +
> + return 0;
> +}
> +
> +static int tsc2007_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device_node *np = client->dev.of_node;
> + struct tsc2007_platform_data *pdata = client->dev.platform_data;
> + struct tsc2007 *ts;
> + struct input_dev *input_dev;
> + int err = 0;
> +
> + ts = devm_kzalloc(&client->dev, sizeof(struct tsc2007), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> +
> + if (np)
> + err = tsc2007_probe_dt(client, ts, np);
> + else
> + err = tsc2007_probe_pdev(client, ts, pdata, id);
> +
> + if (err)
> + return err;
> +
> + if (!i2c_check_functionality(client->adapter,
> + I2C_FUNC_SMBUS_READ_WORD_DATA))
> + return -EIO;
> +
> + input_dev = input_allocate_device();
> + if (!input_dev) {
> + err = -ENOMEM;
> + goto err_free_input;
> + };
> +
> + ts->client = client;
> + ts->irq = client->irq;
> + ts->input = input_dev;
> + init_waitqueue_head(&ts->wait);
> +
> snprintf(ts->phys, sizeof(ts->phys),
> "%s/input0", dev_name(&client->dev));
>
> @@ -331,19 +428,21 @@ static int tsc2007_probe(struct i2c_client *client,
> input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
> input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
>
> - input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, pdata->fuzzx, 0);
> - input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, pdata->fuzzy, 0);
> + input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, ts->fuzzy, 0);
> + input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, ts->fuzzy, 0);
> input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT,
> - pdata->fuzzz, 0);
> + ts->fuzzy, 0);
>
> - if (pdata->init_platform_hw)
> - pdata->init_platform_hw();
> + if (!np) {
> + if (pdata->init_platform_hw)
> + pdata->init_platform_hw();
> + }
>
> err = request_threaded_irq(ts->irq, tsc2007_hard_irq, tsc2007_soft_irq,
> IRQF_ONESHOT, client->dev.driver->name, ts);
> if (err < 0) {
> dev_err(&client->dev, "irq %d busy?\n", ts->irq);
> - goto err_free_mem;
> + goto err_free_input;
> }
>
> tsc2007_stop(ts);
> @@ -358,23 +457,27 @@ static int tsc2007_probe(struct i2c_client *client,
>
> err_free_irq:
> free_irq(ts->irq, ts);
> - if (pdata->exit_platform_hw)
> - pdata->exit_platform_hw();
> - err_free_mem:
> + if (!np) {
> + if (pdata->exit_platform_hw)
> + pdata->exit_platform_hw();
> + }
> + err_free_input:
> input_free_device(input_dev);
> - kfree(ts);
> return err;
> }
>
> static int tsc2007_remove(struct i2c_client *client)
> {
> + struct device_node *np = client->dev.of_node;
> struct tsc2007 *ts = i2c_get_clientdata(client);
> struct tsc2007_platform_data *pdata = client->dev.platform_data;
>
> free_irq(ts->irq, ts);
>
> - if (pdata->exit_platform_hw)
> - pdata->exit_platform_hw();
> + if (!np) {
> + if (pdata->exit_platform_hw)
> + pdata->exit_platform_hw();
> + }
>
> input_unregister_device(ts->input);
> kfree(ts);
> @@ -389,10 +492,19 @@ static const struct i2c_device_id tsc2007_idtable[] = {
>
> MODULE_DEVICE_TABLE(i2c, tsc2007_idtable);
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id tsc2007_of_match[] = {
> + { .compatible = "ti,tsc2007" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, tsc2007_of_match);
> +#endif
> +
> static struct i2c_driver tsc2007_driver = {
> .driver = {
> .owner = THIS_MODULE,
> - .name = "tsc2007"
> + .name = "tsc2007",
> + .of_match_table = of_match_ptr(tsc2007_of_match),
> },
> .id_table = tsc2007_idtable,
> .probe = tsc2007_probe,
> --
> 1.7.9.5
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[-- 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
* Re: [PATCH v2] Input: add regulator haptic driver
From: Oliver Neukum @ 2013-10-24 10:30 UTC (permalink / raw)
To: hyunhee.kim
Cc: 'Dmitry Torokhov', broonie, peter.ujfalusi, wfp5p,
linux-input, linux-kernel, akpm, kyungmin.park,
'Aristeu Sergio Rozanski Filho'
In-Reply-To: <028401ced09b$1f6564d0$5e302e70$%kim@samsung.com>
On Thu, 2013-10-24 at 18:26 +0900, hyunhee.kim wrote:
> Hi,
>
> Thanks for your review.
> I resent patch v3 after removing wrong wrapping.
>
> I made one toggle function because enable/disable functions have redundant codes and another reviewer suggested it.
> Is it better to separate it into two functions?
Linus doesn't like parameters affecting behavior.
Regards
Oliver
^ permalink raw reply
* Re: [PATCHv6][ 1/4] Input: tsc2007: Add device tree support.
From: Rob Herring @ 2013-10-24 10:31 UTC (permalink / raw)
To: Grant Likely
Cc: Denis Carikli, Sascha Hauer, Mark Rutland,
devicetree@vger.kernel.org, Ian Campbell, Pawel Moll,
Stephen Warren, Dmitry Torokhov, Rob Herring, Thierry Reding,
Eric Bénard, linux-input@vger.kernel.org, Shawn Guo,
linux-arm-kernel@lists.infradead.org, Lothar Waßmann
In-Reply-To: <20131024101624.9008BC4039D@trevor.secretlab.ca>
On Thu, Oct 24, 2013 at 5:16 AM, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, 24 Oct 2013 10:38:50 +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 v5->v6:
>> - Fixed a code style issue.
>> - Improved the documentation to set some gpio related properties as optional.
>> - The properties related to the gpios are also explained better.
>> ---
>> .../bindings/input/touchscreen/tsc2007.txt | 44 +++++
>> drivers/input/touchscreen/tsc2007.c | 194 +++++++++++++++-----
>> 2 files changed, 197 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..46fc677
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
>> @@ -0,0 +1,44 @@
>> +* 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).
>> +- interrupt-parent: the phandle for the gpio controller
>> + (see interrupt binding[1]).
>> +- interrupts: (gpio) interrupt to which the chip is connected
>> + (see interrupt binding[1]).
>> +- pinctrl-0: Should specify pin control groups used for the gpio
>> + (see pinctrl bindings[0]).
>> +- pinctrl-names: Should contain only one value - "default"
>> + (see pinctrl bindings[0]).
>> +- max-rt: maximum pressure.
>> +- fuzzy: specifies the fuzz value that is used to filter noise from the event
>> + stream.
>
> I don't actually understand what this means, can you clarify in the
> document?
>
>> +- poll-period: how much time to wait(in millisecond) before reading again the
>> + values from the tsc2007.
>
> max-rt, fuzzy, and poll-period should probably be prefixed with "ti,".
These could be common touchscreen properties. There is also tps6507
touchscreen binding being reviewed. I haven't checked, but there's
probably already some touchscreen bindings. So this is a good
opportunity for common bindings/properties.
Rob
>
> Otherwise the binding looks good to me.
>
> g.
>
>
>> +
>> +[0]: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
>> +[1]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
>> +[2]: Documentation/devicetree/bindings/gpio/gpio.txt
>> +
>> +Example:
>> + &i2c1 {
>> + /* ... */
>> + tsc2007@49 {
>> + compatible = "ti,tsc2007";
>> + reg = <0x49>;
>> + pinctrl-names = "default";
>> + pinctrl-0 = <&pinctrl_tsc2007_1>;
>> + interrupt-parent = <&gpio4>;
>> + interrupts = <0x0 0x8>;
>> + gpios = <&gpio4 0 0>;
>> + ti,x-plate-ohms = <180>;
>> + };
>> +
>> + /* ... */
>> + };
>> diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
>> index 0b67ba4..6b1d7a9 100644
>> --- a/drivers/input/touchscreen/tsc2007.c
>> +++ b/drivers/input/touchscreen/tsc2007.c
>> @@ -26,6 +26,9 @@
>> #include <linux/interrupt.h>
>> #include <linux/i2c.h>
>> #include <linux/i2c/tsc2007.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of.h>
>> +#include <linux/of_gpio.h>
>>
>> #define TSC2007_MEASURE_TEMP0 (0x0 << 4)
>> #define TSC2007_MEASURE_AUX (0x2 << 4)
>> @@ -74,7 +77,10 @@ struct tsc2007 {
>> u16 max_rt;
>> unsigned long poll_delay;
>> unsigned long poll_period;
>> + int fuzzy;
>> + char of;
>>
>> + unsigned gpio;
>> int irq;
>>
>> wait_queue_head_t wait;
>> @@ -84,6 +90,11 @@ struct tsc2007 {
>> void (*clear_penirq)(void);
>> };
>>
>> +static int tsc2007_get_pendown_state_dt(struct tsc2007 *ts)
>> +{
>> + return !gpio_get_value(ts->gpio);
>> +}
>> +
>> static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
>> {
>> s32 data;
>> @@ -142,6 +153,14 @@ static u32 tsc2007_calculate_pressure(struct tsc2007 *tsc, struct ts_event *tc)
>> return rt;
>> }
>>
>> +static bool tsc2007_is_pen_down_valid(struct tsc2007 *ts)
>> +{
>> + if (ts->of)
>> + return gpio_is_valid(ts->gpio);
>> + else
>> + return ts->get_pendown_state ? true : false;
>> +}
>> +
>> static bool tsc2007_is_pen_down(struct tsc2007 *ts)
>> {
>> /*
>> @@ -158,10 +177,13 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts)
>> * to fall back on the pressure reading.
>> */
>>
>> - if (!ts->get_pendown_state)
>> + if (!tsc2007_is_pen_down_valid(ts))
>> return true;
>>
>> - return ts->get_pendown_state();
>> + if (ts->of)
>> + return tsc2007_get_pendown_state_dt(ts);
>> + else
>> + return ts->get_pendown_state();
>> }
>>
>> static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
>> @@ -178,7 +200,7 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
>>
>> rt = tsc2007_calculate_pressure(ts, &tc);
>>
>> - if (rt == 0 && !ts->get_pendown_state) {
>> + if (!rt && !tsc2007_is_pen_down_valid(ts)) {
>> /*
>> * If pressure reported is 0 and we don't have
>> * callback to check pendown state, we have to
>> @@ -228,7 +250,7 @@ static irqreturn_t tsc2007_hard_irq(int irq, void *handle)
>> {
>> struct tsc2007 *ts = handle;
>>
>> - if (!ts->get_pendown_state || likely(ts->get_pendown_state()))
>> + if (tsc2007_is_pen_down(ts))
>> return IRQ_WAKE_THREAD;
>>
>> if (ts->clear_penirq)
>> @@ -273,34 +295,65 @@ static void tsc2007_close(struct input_dev *input_dev)
>> tsc2007_stop(ts);
>> }
>>
>> -static int tsc2007_probe(struct i2c_client *client,
>> - const struct i2c_device_id *id)
>> +#ifdef CONFIG_OF
>> +static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
>> + struct device_node *np)
>> {
>> - struct tsc2007 *ts;
>> - struct tsc2007_platform_data *pdata = client->dev.platform_data;
>> - struct input_dev *input_dev;
>> - int err;
>> -
>> - if (!pdata) {
>> - dev_err(&client->dev, "platform data is required!\n");
>> + int err = 0;
>> + u32 val32;
>> + u64 val64;
>> +
>> + if (!of_property_read_u32(np, "max-rt", &val32))
>> + ts->max_rt = val32;
>> + else
>> + ts->max_rt = MAX_12BIT;
>> +
>> + if (!of_property_read_u32(np, "fuzzy", &val32))
>> + ts->fuzzy = val32;
>> +
>> + if (!of_property_read_u64(np, "poll-period", &val64))
>> + ts->poll_period = val64;
>> + else
>> + ts->poll_period = 1;
>> +
>> + if (!of_property_read_u32(np, "ti,x-plate-ohms", &val32)) {
>> + ts->x_plate_ohms = val32;
>> + } else {
>> + dev_err(&client->dev,
>> + "Error: lacking ti,x-plate-ohms devicetree property. (err %d).",
>> + err);
>> return -EINVAL;
>> }
>>
>> - if (!i2c_check_functionality(client->adapter,
>> - I2C_FUNC_SMBUS_READ_WORD_DATA))
>> - return -EIO;
>> + ts->gpio = of_get_gpio(np, 0);
>> + if (!gpio_is_valid(ts->gpio))
>> + dev_err(&client->dev,
>> + "GPIO not found (of_get_gpio returned %d)\n",
>> + ts->gpio);
>>
>> - ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL);
>> - input_dev = input_allocate_device();
>> - if (!ts || !input_dev) {
>> - err = -ENOMEM;
>> - goto err_free_mem;
>> - }
>> + /* Used to detect if it is probed trough the device tree,
>> + * in order to be able to use that information in the IRQ handler.
>> + */
>> + ts->of = 1;
>>
>> - ts->client = client;
>> - ts->irq = client->irq;
>> - ts->input = input_dev;
>> - init_waitqueue_head(&ts->wait);
>> + return 0;
>> +}
>> +#else
>> +static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
>> + struct device_node *np)
>> +{
>> + return -ENODEV;
>> +}
>> +#endif
>> +
>> +static int tsc2007_probe_pdev(struct i2c_client *client, struct tsc2007 *ts,
>> + struct tsc2007_platform_data *pdata,
>> + const struct i2c_device_id *id)
>> +{
>> + if (!pdata) {
>> + dev_err(&client->dev, "platform data is required!\n");
>> + return -EINVAL;
>> + }
>>
>> ts->model = pdata->model;
>> ts->x_plate_ohms = pdata->x_plate_ohms;
>> @@ -309,13 +362,57 @@ static int tsc2007_probe(struct i2c_client *client,
>> ts->poll_period = pdata->poll_period ? : 1;
>> ts->get_pendown_state = pdata->get_pendown_state;
>> ts->clear_penirq = pdata->clear_penirq;
>> + ts->fuzzy = pdata->fuzzy;
>>
>> if (pdata->x_plate_ohms == 0) {
>> dev_err(&client->dev, "x_plate_ohms is not set up in platform data");
>> - err = -EINVAL;
>> - goto err_free_mem;
>> + return -EINVAL;
>> }
>>
>> + /* Used to detect if it is probed trough the device tree,
>> + * in order to be able to use that information in the IRQ handler.
>> + */
>> + ts->of = 0;
>> +
>> + return 0;
>> +}
>> +
>> +static int tsc2007_probe(struct i2c_client *client,
>> + const struct i2c_device_id *id)
>> +{
>> + struct device_node *np = client->dev.of_node;
>> + struct tsc2007_platform_data *pdata = client->dev.platform_data;
>> + struct tsc2007 *ts;
>> + struct input_dev *input_dev;
>> + int err = 0;
>> +
>> + ts = devm_kzalloc(&client->dev, sizeof(struct tsc2007), GFP_KERNEL);
>> + if (!ts)
>> + return -ENOMEM;
>> +
>> + if (np)
>> + err = tsc2007_probe_dt(client, ts, np);
>> + else
>> + err = tsc2007_probe_pdev(client, ts, pdata, id);
>> +
>> + if (err)
>> + return err;
>> +
>> + if (!i2c_check_functionality(client->adapter,
>> + I2C_FUNC_SMBUS_READ_WORD_DATA))
>> + return -EIO;
>> +
>> + input_dev = input_allocate_device();
>> + if (!input_dev) {
>> + err = -ENOMEM;
>> + goto err_free_input;
>> + };
>> +
>> + ts->client = client;
>> + ts->irq = client->irq;
>> + ts->input = input_dev;
>> + init_waitqueue_head(&ts->wait);
>> +
>> snprintf(ts->phys, sizeof(ts->phys),
>> "%s/input0", dev_name(&client->dev));
>>
>> @@ -331,19 +428,21 @@ static int tsc2007_probe(struct i2c_client *client,
>> input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
>> input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
>>
>> - input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, pdata->fuzzx, 0);
>> - input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, pdata->fuzzy, 0);
>> + input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, ts->fuzzy, 0);
>> + input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, ts->fuzzy, 0);
>> input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT,
>> - pdata->fuzzz, 0);
>> + ts->fuzzy, 0);
>>
>> - if (pdata->init_platform_hw)
>> - pdata->init_platform_hw();
>> + if (!np) {
>> + if (pdata->init_platform_hw)
>> + pdata->init_platform_hw();
>> + }
>>
>> err = request_threaded_irq(ts->irq, tsc2007_hard_irq, tsc2007_soft_irq,
>> IRQF_ONESHOT, client->dev.driver->name, ts);
>> if (err < 0) {
>> dev_err(&client->dev, "irq %d busy?\n", ts->irq);
>> - goto err_free_mem;
>> + goto err_free_input;
>> }
>>
>> tsc2007_stop(ts);
>> @@ -358,23 +457,27 @@ static int tsc2007_probe(struct i2c_client *client,
>>
>> err_free_irq:
>> free_irq(ts->irq, ts);
>> - if (pdata->exit_platform_hw)
>> - pdata->exit_platform_hw();
>> - err_free_mem:
>> + if (!np) {
>> + if (pdata->exit_platform_hw)
>> + pdata->exit_platform_hw();
>> + }
>> + err_free_input:
>> input_free_device(input_dev);
>> - kfree(ts);
>> return err;
>> }
>>
>> static int tsc2007_remove(struct i2c_client *client)
>> {
>> + struct device_node *np = client->dev.of_node;
>> struct tsc2007 *ts = i2c_get_clientdata(client);
>> struct tsc2007_platform_data *pdata = client->dev.platform_data;
>>
>> free_irq(ts->irq, ts);
>>
>> - if (pdata->exit_platform_hw)
>> - pdata->exit_platform_hw();
>> + if (!np) {
>> + if (pdata->exit_platform_hw)
>> + pdata->exit_platform_hw();
>> + }
>>
>> input_unregister_device(ts->input);
>> kfree(ts);
>> @@ -389,10 +492,19 @@ static const struct i2c_device_id tsc2007_idtable[] = {
>>
>> MODULE_DEVICE_TABLE(i2c, tsc2007_idtable);
>>
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id tsc2007_of_match[] = {
>> + { .compatible = "ti,tsc2007" },
>> + { /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(of, tsc2007_of_match);
>> +#endif
>> +
>> static struct i2c_driver tsc2007_driver = {
>> .driver = {
>> .owner = THIS_MODULE,
>> - .name = "tsc2007"
>> + .name = "tsc2007",
>> + .of_match_table = of_match_ptr(tsc2007_of_match),
>> },
>> .id_table = tsc2007_idtable,
>> .probe = tsc2007_probe,
>> --
>> 1.7.9.5
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
--
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
* [PATCHv7][ 1/4] Input: tsc2007: Add device tree support.
From: Denis Carikli @ 2013-10-24 12:42 UTC (permalink / raw)
To: Rob Herring
Cc: Shawn Guo, Eric Bénard, Sascha Hauer, linux-arm-kernel,
Grant Likely, Thierry Reding, Denis Carikli, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, devicetree,
Dmitry Torokhov, linux-input, Lothar Waßmann
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).
+- interrupt-parent: the phandle for the gpio controller
+ (see interrupt binding[1]).
+- interrupts: (gpio) interrupt to which the chip is connected
+ (see interrupt binding[1]).
+- pinctrl-0: Should specify pin control groups used for the gpio
+ (see pinctrl bindings[0]).
+- pinctrl-names: Should contain only one value - "default"
+ (see pinctrl bindings[0]).
+- ti,max-rt: maximum pressure.
+- ti,fuzzy: specifies the absolute input fuzz value.
+ If set, it will permit noise in the data up to +- the value given to the fuzz
+ parameter, that is used to filter noise from the event stream.
+- ti,poll-period: how much time to wait(in millisecond) before reading again the
+ values from the tsc2007.
+
+[0]: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+[1]: Documentation/devicetree/bindings/interrupt-controller/interrupts.txt
+[2]: Documentation/devicetree/bindings/gpio/gpio.txt
+
+Example:
+ &i2c1 {
+ /* ... */
+ tsc2007@49 {
+ compatible = "ti,tsc2007";
+ reg = <0x49>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_tsc2007_1>;
+ interrupt-parent = <&gpio4>;
+ interrupts = <0x0 0x8>;
+ gpios = <&gpio4 0 0>;
+ ti,x-plate-ohms = <180>;
+ };
+
+ /* ... */
+ };
diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
index 0b67ba4..c50d64e 100644
--- a/drivers/input/touchscreen/tsc2007.c
+++ b/drivers/input/touchscreen/tsc2007.c
@@ -26,6 +26,9 @@
#include <linux/interrupt.h>
#include <linux/i2c.h>
#include <linux/i2c/tsc2007.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
#define TSC2007_MEASURE_TEMP0 (0x0 << 4)
#define TSC2007_MEASURE_AUX (0x2 << 4)
@@ -74,7 +77,10 @@ struct tsc2007 {
u16 max_rt;
unsigned long poll_delay;
unsigned long poll_period;
+ int fuzzy;
+ char of;
+ unsigned gpio;
int irq;
wait_queue_head_t wait;
@@ -84,6 +90,11 @@ struct tsc2007 {
void (*clear_penirq)(void);
};
+static int tsc2007_get_pendown_state_dt(struct tsc2007 *ts)
+{
+ return !gpio_get_value(ts->gpio);
+}
+
static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
{
s32 data;
@@ -142,6 +153,14 @@ static u32 tsc2007_calculate_pressure(struct tsc2007 *tsc, struct ts_event *tc)
return rt;
}
+static bool tsc2007_is_pen_down_valid(struct tsc2007 *ts)
+{
+ if (ts->of)
+ return gpio_is_valid(ts->gpio);
+ else
+ return ts->get_pendown_state ? true : false;
+}
+
static bool tsc2007_is_pen_down(struct tsc2007 *ts)
{
/*
@@ -158,10 +177,13 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts)
* to fall back on the pressure reading.
*/
- if (!ts->get_pendown_state)
+ if (!tsc2007_is_pen_down_valid(ts))
return true;
- return ts->get_pendown_state();
+ if (ts->of)
+ return tsc2007_get_pendown_state_dt(ts);
+ else
+ return ts->get_pendown_state();
}
static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
@@ -178,7 +200,7 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
rt = tsc2007_calculate_pressure(ts, &tc);
- if (rt == 0 && !ts->get_pendown_state) {
+ if (!rt && !tsc2007_is_pen_down_valid(ts)) {
/*
* If pressure reported is 0 and we don't have
* callback to check pendown state, we have to
@@ -228,7 +250,7 @@ static irqreturn_t tsc2007_hard_irq(int irq, void *handle)
{
struct tsc2007 *ts = handle;
- if (!ts->get_pendown_state || likely(ts->get_pendown_state()))
+ if (tsc2007_is_pen_down(ts))
return IRQ_WAKE_THREAD;
if (ts->clear_penirq)
@@ -273,34 +295,65 @@ static void tsc2007_close(struct input_dev *input_dev)
tsc2007_stop(ts);
}
-static int tsc2007_probe(struct i2c_client *client,
- const struct i2c_device_id *id)
+#ifdef CONFIG_OF
+static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
+ struct device_node *np)
{
- struct tsc2007 *ts;
- struct tsc2007_platform_data *pdata = client->dev.platform_data;
- struct input_dev *input_dev;
- int err;
-
- if (!pdata) {
- dev_err(&client->dev, "platform data is required!\n");
+ int err = 0;
+ u32 val32;
+ u64 val64;
+
+ if (!of_property_read_u32(np, "ti,max-rt", &val32))
+ ts->max_rt = val32;
+ else
+ ts->max_rt = MAX_12BIT;
+
+ if (!of_property_read_u32(np, "ti,fuzzy", &val32))
+ ts->fuzzy = val32;
+
+ if (!of_property_read_u64(np, "ti,poll-period", &val64))
+ ts->poll_period = val64;
+ else
+ ts->poll_period = 1;
+
+ if (!of_property_read_u32(np, "ti,x-plate-ohms", &val32)) {
+ ts->x_plate_ohms = val32;
+ } else {
+ dev_err(&client->dev,
+ "Error: lacking ti,x-plate-ohms devicetree property. (err %d).",
+ err);
return -EINVAL;
}
- if (!i2c_check_functionality(client->adapter,
- I2C_FUNC_SMBUS_READ_WORD_DATA))
- return -EIO;
+ ts->gpio = of_get_gpio(np, 0);
+ if (!gpio_is_valid(ts->gpio))
+ dev_err(&client->dev,
+ "GPIO not found (of_get_gpio returned %d)\n",
+ ts->gpio);
- ts = kzalloc(sizeof(struct tsc2007), GFP_KERNEL);
- input_dev = input_allocate_device();
- if (!ts || !input_dev) {
- err = -ENOMEM;
- goto err_free_mem;
- }
+ /* Used to detect if it is probed trough the device tree,
+ * in order to be able to use that information in the IRQ handler.
+ */
+ ts->of = 1;
- ts->client = client;
- ts->irq = client->irq;
- ts->input = input_dev;
- init_waitqueue_head(&ts->wait);
+ return 0;
+}
+#else
+static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
+ struct device_node *np)
+{
+ return -ENODEV;
+}
+#endif
+
+static int tsc2007_probe_pdev(struct i2c_client *client, struct tsc2007 *ts,
+ struct tsc2007_platform_data *pdata,
+ const struct i2c_device_id *id)
+{
+ if (!pdata) {
+ dev_err(&client->dev, "platform data is required!\n");
+ return -EINVAL;
+ }
ts->model = pdata->model;
ts->x_plate_ohms = pdata->x_plate_ohms;
@@ -309,13 +362,57 @@ static int tsc2007_probe(struct i2c_client *client,
ts->poll_period = pdata->poll_period ? : 1;
ts->get_pendown_state = pdata->get_pendown_state;
ts->clear_penirq = pdata->clear_penirq;
+ ts->fuzzy = pdata->fuzzy;
if (pdata->x_plate_ohms == 0) {
dev_err(&client->dev, "x_plate_ohms is not set up in platform data");
- err = -EINVAL;
- goto err_free_mem;
+ return -EINVAL;
}
+ /* Used to detect if it is probed trough the device tree,
+ * in order to be able to use that information in the IRQ handler.
+ */
+ ts->of = 0;
+
+ return 0;
+}
+
+static int tsc2007_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct device_node *np = client->dev.of_node;
+ struct tsc2007_platform_data *pdata = client->dev.platform_data;
+ struct tsc2007 *ts;
+ struct input_dev *input_dev;
+ int err = 0;
+
+ ts = devm_kzalloc(&client->dev, sizeof(struct tsc2007), GFP_KERNEL);
+ if (!ts)
+ return -ENOMEM;
+
+ if (np)
+ err = tsc2007_probe_dt(client, ts, np);
+ else
+ err = tsc2007_probe_pdev(client, ts, pdata, id);
+
+ if (err)
+ return err;
+
+ if (!i2c_check_functionality(client->adapter,
+ I2C_FUNC_SMBUS_READ_WORD_DATA))
+ return -EIO;
+
+ input_dev = input_allocate_device();
+ if (!input_dev) {
+ err = -ENOMEM;
+ goto err_free_input;
+ };
+
+ ts->client = client;
+ ts->irq = client->irq;
+ ts->input = input_dev;
+ init_waitqueue_head(&ts->wait);
+
snprintf(ts->phys, sizeof(ts->phys),
"%s/input0", dev_name(&client->dev));
@@ -331,19 +428,21 @@ static int tsc2007_probe(struct i2c_client *client,
input_dev->evbit[0] = BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
input_dev->keybit[BIT_WORD(BTN_TOUCH)] = BIT_MASK(BTN_TOUCH);
- input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, pdata->fuzzx, 0);
- input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, pdata->fuzzy, 0);
+ input_set_abs_params(input_dev, ABS_X, 0, MAX_12BIT, ts->fuzzy, 0);
+ input_set_abs_params(input_dev, ABS_Y, 0, MAX_12BIT, ts->fuzzy, 0);
input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT,
- pdata->fuzzz, 0);
+ ts->fuzzy, 0);
- if (pdata->init_platform_hw)
- pdata->init_platform_hw();
+ if (!np) {
+ if (pdata->init_platform_hw)
+ pdata->init_platform_hw();
+ }
err = request_threaded_irq(ts->irq, tsc2007_hard_irq, tsc2007_soft_irq,
IRQF_ONESHOT, client->dev.driver->name, ts);
if (err < 0) {
dev_err(&client->dev, "irq %d busy?\n", ts->irq);
- goto err_free_mem;
+ goto err_free_input;
}
tsc2007_stop(ts);
@@ -358,23 +457,27 @@ static int tsc2007_probe(struct i2c_client *client,
err_free_irq:
free_irq(ts->irq, ts);
- if (pdata->exit_platform_hw)
- pdata->exit_platform_hw();
- err_free_mem:
+ if (!np) {
+ if (pdata->exit_platform_hw)
+ pdata->exit_platform_hw();
+ }
+ err_free_input:
input_free_device(input_dev);
- kfree(ts);
return err;
}
static int tsc2007_remove(struct i2c_client *client)
{
+ struct device_node *np = client->dev.of_node;
struct tsc2007 *ts = i2c_get_clientdata(client);
struct tsc2007_platform_data *pdata = client->dev.platform_data;
free_irq(ts->irq, ts);
- if (pdata->exit_platform_hw)
- pdata->exit_platform_hw();
+ if (!np) {
+ if (pdata->exit_platform_hw)
+ pdata->exit_platform_hw();
+ }
input_unregister_device(ts->input);
kfree(ts);
@@ -389,10 +492,19 @@ static const struct i2c_device_id tsc2007_idtable[] = {
MODULE_DEVICE_TABLE(i2c, tsc2007_idtable);
+#ifdef CONFIG_OF
+static const struct of_device_id tsc2007_of_match[] = {
+ { .compatible = "ti,tsc2007" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, tsc2007_of_match);
+#endif
+
static struct i2c_driver tsc2007_driver = {
.driver = {
.owner = THIS_MODULE,
- .name = "tsc2007"
+ .name = "tsc2007",
+ .of_match_table = of_match_ptr(tsc2007_of_match),
},
.id_table = tsc2007_idtable,
.probe = tsc2007_probe,
--
1.7.9.5
--
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
* [PATCHv7][ 2/4] ARM: dts: cpuimx51 Add touchscreen support.
From: Denis Carikli @ 2013-10-24 12:42 UTC (permalink / raw)
To: Rob Herring
Cc: Shawn Guo, Eric Bénard, Sascha Hauer, linux-arm-kernel,
Grant Likely, Thierry Reding, Denis Carikli, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, devicetree,
Dmitry Torokhov, linux-input, Lothar Waßmann
In-Reply-To: <1382618536-25320-1-git-send-email-denis@eukrea.com>
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>
---
arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi b/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
index 8638656..34ca8d3a 100644
--- a/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
+++ b/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
@@ -42,6 +42,17 @@
compatible = "nxp,pcf8563";
reg = <0x51>;
};
+
+ tsc2007: tsc2007@49 {
+ compatible = "ti,tsc2007";
+ reg = <0x49>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_tsc2007_1>;
+ interrupt-parent = <&gpio4>;
+ interrupts = <0x0 0x8>;
+ gpios = <&gpio4 0 0>;
+ ti,x-plate-ohms = <180>;
+ };
};
&iomuxc {
--
1.7.9.5
--
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
* [PATCHv7][ 3/4] ARM: dts: cpuimx35 Add touchscreen support.
From: Denis Carikli @ 2013-10-24 12:42 UTC (permalink / raw)
To: Rob Herring
Cc: Shawn Guo, Eric Bénard, Sascha Hauer, linux-arm-kernel,
Grant Likely, Thierry Reding, Denis Carikli, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, devicetree,
Dmitry Torokhov, linux-input, Lothar Waßmann
In-Reply-To: <1382618536-25320-1-git-send-email-denis@eukrea.com>
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>
---
arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
index 2c2d4cd..a22230b 100644
--- a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
+++ b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
@@ -41,6 +41,17 @@
compatible = "nxp,pcf8563";
reg = <0x51>;
};
+
+ tsc2007: tsc2007@48 {
+ compatible = "ti,tsc2007";
+ reg = <0x48>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_tsc2007_1>;
+ interrupt-parent = <&gpio3>;
+ interrupts = <0x2 0x8>;
+ gpios = <&gpio3 2 0>;
+ ti,x-plate-ohms = <180>;
+ };
};
&iomuxc {
--
1.7.9.5
--
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
* [PATCHv7][ 4/4] ARM: imx_v6_v7_defconfig: Enable tsc2007 support.
From: Denis Carikli @ 2013-10-24 12:42 UTC (permalink / raw)
To: Rob Herring
Cc: Shawn Guo, Eric Bénard, Sascha Hauer, linux-arm-kernel,
Grant Likely, Thierry Reding, Denis Carikli, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, devicetree,
Dmitry Torokhov, linux-input, Lothar Waßmann, Fabio Estevam
In-Reply-To: <1382618536-25320-1-git-send-email-denis@eukrea.com>
The eukrea cpuimx35 and cpuimx51 have a tsc2007 touchscreen controller,
so we turn it on.
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: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
arch/arm/configs/imx_v6_v7_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/configs/imx_v6_v7_defconfig b/arch/arm/configs/imx_v6_v7_defconfig
index d8a52a0..23ba17d 100644
--- a/arch/arm/configs/imx_v6_v7_defconfig
+++ b/arch/arm/configs/imx_v6_v7_defconfig
@@ -130,6 +130,7 @@ CONFIG_MOUSE_PS2_ELANTECH=y
CONFIG_INPUT_TOUCHSCREEN=y
CONFIG_TOUCHSCREEN_EGALAX=y
CONFIG_TOUCHSCREEN_MC13783=y
+CONFIG_TOUCHSCREEN_TSC2007=y
CONFIG_INPUT_MISC=y
CONFIG_INPUT_MMA8450=y
CONFIG_SERIO_SERPORT=m
--
1.7.9.5
--
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
* Re: [PATCH] Input: add regulator haptic driver
From: Mark Brown @ 2013-10-24 13:38 UTC (permalink / raw)
To: hyunhee.kim
Cc: dmitry.torokhov, peter.ujfalusi, wfp5p, linux-input, linux-kernel,
akpm
In-Reply-To: <000101cec628$aaff69a0$00fe3ce0$%kim@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 1607 bytes --]
On Fri, Oct 11, 2013 at 11:22:00AM +0900, hyunhee.kim wrote:
> + if (enable && !haptic->enabled) {
> + haptic->enabled = true;
> + ret = regulator_enable(haptic->regulator);
> + if (ret)
> + pr_err("haptic: %s failed to enable regulator\n",
> + __func__);
These should probably set the flag after the regulator API call has
succeeded, otherwise if the call fails the driver will incorrectly
remember that the regulator is enabled and never disable it (or vice
versa on disable).
> +static void regulator_haptic_work(struct work_struct *work)
> +{
> + struct regulator_haptic *haptic = container_of(work,
> + struct
> regulator_haptic,
> + work);
> + if (haptic->level)
> + regulator_haptic_enable(haptic, true);
> + else
> + regulator_haptic_enable(haptic, false);
> +
> +}
Should the mutex for the level be at this level rather than in the
subfunctions? Though it'd probably be just as well to inline the true
and false cases since there's basically two equivalent forks in the
enable function anyway.
> +
> + haptic = kzalloc(sizeof(*haptic), GFP_KERNEL);
> + if (!haptic) {
> + dev_err(&pdev->dev, "unable to allocate memory for
> haptic\n");
> + return -ENOMEM;
> + }
Might be better to use devm_ functions throughout here, saves error
handling and cleanup code...
> +static int regulator_haptic_remove(struct platform_device *pdev)
> +{
> + struct regulator_haptic *haptic = platform_get_drvdata(pdev);
> +
> + input_unregister_device(haptic->input_dev);
> +
> + return 0;
...which is currently missing.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* [PATCHv6 0/3] DT Support for TWL4030 power button
From: Sebastian Reichel @ 2013-10-24 14:48 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Torokhov
Cc: Grant Likely, Rob Herring, Sebastian Reichel, Peter Ujfalusi,
Sachin Kamat, Florian Vaussard, linux-input, linux-kernel,
devicetree
Hi,
This is the sixth iteration of DT support for the TWL4030
power button.
Changes since v5 [0]:
* Updated documentation
[0] https://lkml.org/lkml/2013/10/23/416
-- Sebastian
Sebastian Reichel (3):
Input: twl4030-pwrbutton - add device tree support
Input: twl4030-pwrbutton: use dev_err for errors
Input: twl4030-pwrbutton: simplify driver using devm_*
.../bindings/input/twl4030-pwrbutton.txt | 21 +++++++++++
drivers/input/misc/twl4030-pwrbutton.c | 44 +++++++++-------------
2 files changed, 38 insertions(+), 27 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
--
1.8.4.rc3
^ permalink raw reply
* [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Sebastian Reichel @ 2013-10-24 14:48 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Torokhov
Cc: Grant Likely, Rob Herring, Sebastian Reichel, Peter Ujfalusi,
Sachin Kamat, Florian Vaussard,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1382626126-12565-1-git-send-email-sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
Add device tree support for twl4030 power button driver.
Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.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
+
+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),
.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 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
* [PATCHv6 2/3] Input: twl4030-pwrbutton: use dev_err for errors
From: Sebastian Reichel @ 2013-10-24 14:48 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Torokhov
Cc: Grant Likely, Rob Herring, Sebastian Reichel, Peter Ujfalusi,
Sachin Kamat, Florian Vaussard, linux-input, linux-kernel,
devicetree
In-Reply-To: <1382626126-12565-1-git-send-email-sre@debian.org>
Use dev_err() to output errors instead of dev_dbg().
Signed-off-by: Sebastian Reichel <sre@debian.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
^ permalink raw reply related
* [PATCHv6 3/3] Input: twl4030-pwrbutton: simplify driver using devm_*
From: Sebastian Reichel @ 2013-10-24 14:48 UTC (permalink / raw)
To: Sebastian Reichel, Dmitry Torokhov
Cc: Grant Likely, Rob Herring, Sebastian Reichel, Peter Ujfalusi,
Sachin Kamat, Florian Vaussard, linux-input, linux-kernel,
devicetree
In-Reply-To: <1382626126-12565-1-git-send-email-sre@debian.org>
Use managed irq resource to simplify the driver.
Signed-off-by: Sebastian Reichel <sre@debian.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
^ permalink raw reply related
* Re: [PATCHv7][ 1/4] Input: tsc2007: Add device tree support.
From: Thierry Reding @ 2013-10-24 15:17 UTC (permalink / raw)
To: Denis Carikli
Cc: Rob Herring, Shawn Guo, Eric Bénard, Sascha Hauer,
linux-arm-kernel, Grant Likely, Pawel Moll, Mark Rutland,
Stephen Warren, Ian Campbell, devicetree, Dmitry Torokhov,
linux-input, Lothar Waßmann, Linus Walleij
In-Reply-To: <1382618536-25320-1-git-send-email-denis@eukrea.com>
[-- Attachment #1: Type: text/plain, Size: 3421 bytes --]
On Thu, Oct 24, 2013 at 02:42:13PM +0200, Denis Carikli 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).
> +- interrupt-parent: the phandle for the gpio controller
> + (see interrupt binding[1]).
> +- interrupts: (gpio) interrupt to which the chip is connected
> + (see interrupt binding[1]).
> +- pinctrl-0: Should specify pin control groups used for the gpio
> + (see pinctrl bindings[0]).
> +- pinctrl-names: Should contain only one value - "default"
> + (see pinctrl bindings[0]).
Also I haven't seen a response as to why this can't be handled by the
GPIO driver. Adding Linus Walleij, perhaps he knows a more definitive
answer.
Linus, the issue here is that the pinctrl properties for this chip are
supposed to pinmux the pendown GPIO for this chip. I was under the
impression that this should be handled by the GPIO controller itself, so
that when gpio_request() was called on a pin it would be the GPIO
controller driver's responsibility to pinmux it appropriately.
> +- ti,max-rt: maximum pressure.
> +- ti,fuzzy: specifies the absolute input fuzz value.
> + If set, it will permit noise in the data up to +- the value given to the fuzz
> + parameter, that is used to filter noise from the event stream.
Any reason why these can't be specified per axis? From my experience
with this chip it can make sense to use different values for the
different axes. Well, perhaps X and Y axes can share one fuzz factor,
but the Z axis is sometimes different.
You also modify the driver to ignore the X and Z axes fuzz factor even
for platform data where specifying all three separately was explicitly
allowed. Have you verified that this doesn't cause any regressions?
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCHv7][ 1/4] Input: tsc2007: Add device tree support.
From: Eric Bénard @ 2013-10-24 15:26 UTC (permalink / raw)
To: Thierry Reding
Cc: Denis Carikli, Mark Rutland, devicetree, Dmitry Torokhov,
Pawel Moll, Stephen Warren, Linus Walleij, Ian Campbell,
Rob Herring, Grant Likely, Sascha Hauer, linux-input, Shawn Guo,
linux-arm-kernel, Lothar Waßmann
In-Reply-To: <20131024151736.GE9044@ulmo.nvidia.com>
Hi Thierry,
Le Thu, 24 Oct 2013 17:17:37 +0200,
Thierry Reding <thierry.reding@gmail.com> a écrit :
> > +- ti,max-rt: maximum pressure.
> > +- ti,fuzzy: specifies the absolute input fuzz value.
> > + If set, it will permit noise in the data up to +- the value given to the fuzz
> > + parameter, that is used to filter noise from the event stream.
>
> Any reason why these can't be specified per axis? From my experience
> with this chip it can make sense to use different values for the
> different axes. Well, perhaps X and Y axes can share one fuzz factor,
> but the Z axis is sometimes different.
>
> You also modify the driver to ignore the X and Z axes fuzz factor even
> for platform data where specifying all three separately was explicitly
> allowed. Have you verified that this doesn't cause any regressions?
>
That was not intended, that's an error, we will fix that.
Thanks,
Eric
--
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
* Re: [PATCHv7][ 1/4] Input: tsc2007: Add device tree support.
From: Grant Likely @ 2013-10-24 16:47 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, Dmitry Torokhov, Pawel Moll,
Stephen Warren, Ian Campbell, Denis Carikli, Thierry Reding,
Sascha Hauer, linux-input, Shawn Guo, linux-arm-kernel
In-Reply-To: <1382618536-25320-1-git-send-email-denis@eukrea.com>
[-- Attachment #1: Type: text/plain, Size: 1965 bytes --]
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.
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
* Re: [PATCH V4 1/2] tps6507x-ts: Add DT support
From: Manish Badarkhe @ 2013-10-24 17:05 UTC (permalink / raw)
To: Mark Rutland
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
davinci-linux-open-source-VycZQUHpC/PFrsHnngEfi1aTQe2KTcn/@public.gmane.org,
linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org,
grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org,
rob-VoJi6FS/r0vR7s880joybQ@public.gmane.org,
linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20131023164548.GB6042@kartoffel>
[-- Attachment #1.1: Type: text/plain, Size: 8199 bytes --]
Hi Mark,
Thank you for your reply.
On Wed, Oct 23, 2013 at 10:15 PM, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> On Wed, Oct 23, 2013 at 05:28:52PM +0100, Manish Badarkhe wrote:
> > Add device tree based support for TI's tps6507x touchscreen.
> >
> > Signed-off-by: Manish Badarkhe <badarkhe.manish-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> > Changes since V3:
> > - Rebased on top of Dmitry's changes
> > - Removed error handling for optional DT properties
> >
> > Changes since V2:
> > - Removed unnecessary code.
> > - Updated Documentation to provide proper names of
> > devicetree properties.
> >
> > Changes since V1:
> > - Updated documentation to specify tps6507x as multifunctional
> > device.
> > - return proper error value in absence of platform and DT
> > data for touchscreen.
> > - Updated commit message.
> >
> > :100755 100755 8fffa3c... e1b9cfd... M
> Documentation/devicetree/bindings/mfd/tps6507x.txt
> > :100644 100644 db604e0... 0cf0eb1... M
> drivers/input/touchscreen/tps6507x-ts.c
> > Documentation/devicetree/bindings/mfd/tps6507x.txt | 24 ++++++-
> > drivers/input/touchscreen/tps6507x-ts.c | 72
> ++++++++++++--------
> > 2 files changed, 67 insertions(+), 29 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt
> b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> > index 8fffa3c..e1b9cfd 100755
> > --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
> > +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> > @@ -1,4 +1,8 @@
> > -TPS6507x Power Management Integrated Circuit
> > +TPS6507x Multifunctional Device.
> > +
> > +Features provided by TPS6507x:
> > + 1.Power Management Integrated Circuit.
> > + 2.Touch-Screen.
> >
> > Required properties:
> > - compatible: "ti,tps6507x"
> > @@ -23,6 +27,9 @@ Required properties:
> > vindcdc1_2-supply: VDCDC1 and VDCDC2 input.
> > vindcdc3-supply : VDCDC3 input.
> > vldo1_2-supply : VLDO1 and VLDO2 input.
> > +- tsc: This node specifies touch screen data.
>
> This is a node, but is listed un "Required properties". That seems odd.
>
> When is it required?
>
> Why is the data under the tsc subnode? Why not directly under the main
> node?
>
Yes, It seems odd to add a subnode properties here. I gone through other
documentation
for MFD devices and observed that separate documentation file is present
for sub node modules.
TPS6507x is multifunctional device and having touch screen submodule. As it
is child node for
TPS6507x multifunctional device, I have created child node as "tsc".
>
> > + ti,poll-period : Time at which touch input is getting sampled in
> ms.
>
> Is this for debounce? Other bindings have similar but differently named
> properties.
>
> This is not debounce time. This time is required for input poll devices.
> Why is this necessary? Can the driver not choose a sane value?
>
> poll-period is necessary to sample touch inputs. Driver will chose value
default poll
period if this value is not present. I was bit confused here, whether to
add this property
as optional or required. As with default period touch not achieve
performance.
Can I make this property optional?
> > + ti,min-pressure: Minimum pressure value to trigger touch.
>
> What type are these? Single (u32) cells?
>
> What units is ti,min-pressure in?
>
> No, its a u16 cell. It is measured in ohm. Again default value for
min-pressure is available
in driver code. Can I make this property optional?
>
> > Regulator Optional properties:
> > - defdcdc_default: It's property of DCDC2 and DCDC3 regulators.
> > @@ -30,6 +37,14 @@ Regulator Optional properties:
> > 1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
> > If this property is not defined, it defaults to 0 (not enabled).
> >
> > +Touchscreen Optional properties:
> > +- ti,vendor : Touchscreen vendor id to populate
> > + in sysclass interface.
> > +- ti,product: Touchscreen product id to populate
> > + in sysclass interface.
> > +- ti,version: Touchscreen version id to populate
> > + in sysclass interface.
>
> Are these integers, strings, or something else?
>
> These are unsigned short(u16) values.
> What values make sense?
>
> These values are only to tell about chip details.
> What's the sysclass interface? That sounds like a Linux detail. The
> properties
> might be fine but the description shouldn't need to refer to Linux
> internals.
>
> Okay, I will update description for these properties.
> +
> > Example:
> >
> > pmu: tps6507x@48 {
> > @@ -88,4 +103,11 @@ Example:
> > };
> > };
> >
> > + tsc {
> > + ti,poll-period = <30>;
> > + ti,min-pressure = <0x30>;
> > + ti,vendor = <0>;
> > + ti,product = <65070>;
> > + ti,version = <0x100>;
> > + };
> > };
> > diff --git a/drivers/input/touchscreen/tps6507x-ts.c
> b/drivers/input/touchscreen/tps6507x-ts.c
> > index db604e0..0cf0eb1 100644
> > --- a/drivers/input/touchscreen/tps6507x-ts.c
> > +++ b/drivers/input/touchscreen/tps6507x-ts.c
> > @@ -22,6 +22,8 @@
> > #include <linux/mfd/tps6507x.h>
> > #include <linux/input/tps6507x-ts.h>
> > #include <linux/delay.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> >
> > #define TSC_DEFAULT_POLL_PERIOD 30 /* ms */
> > #define TPS_DEFAULT_MIN_PRESSURE 0x30
> > @@ -206,33 +208,54 @@ done:
> > tps6507x_adc_standby(tsc);
> > }
> >
> > +static void tsc_init_data(struct tps6507x_ts *tsc,
> > + struct input_dev *input_dev)
> > +{
> > + struct device_node *node = tsc->dev->of_node;
> > + const struct tps6507x_board *tps_board =
> > + dev_get_platdata(tsc->dev);
> > + const struct touchscreen_init_data *init_data = NULL;
> > +
> > + if (node)
> > + node = of_find_node_by_name(node, "tsc");
>
> As of_find_node_by_name traverses the list of all nodes (not just
> children),
> this may match nodes other than what you expect.
>
I think, It traverse nodes from parent node (i.e. tps6507x) and find child.
Please correct me if I am wrong?
>
> > + if (tps_board)
> > + /*
> > + * init_data points to array of touchscreen_init structure
> > + * coming from the board-evm file.
> > + */
> > + init_data = tps_board->tps6507x_ts_init_data;
> > +
> > + if (node == NULL || init_data == NULL) {
> > + tsc->poll_dev->poll_interval = TSC_DEFAULT_POLL_PERIOD;
> > + tsc->min_pressure = TPS_DEFAULT_MIN_PRESSURE;
> > + } else if (init_data) {
> > + tsc->poll_dev->poll_interval = init_data->poll_period;
> > + tsc->min_pressure = init_data->min_pressure;
> > + input_dev->id.vendor = init_data->vendor;
> > + input_dev->id.product = init_data->product;
> > + input_dev->id.version = init_data->version;
> > + } else if (node) {
> > + of_property_read_u32(node, "ti,poll-period",
> > + &tsc->poll_dev->poll_interval);
> > + of_property_read_u16(node, "ti,min-pressure",
> > + &tsc->min_pressure);
>
> You didn't mention these were 16-bit values in the binding.
>
> As DTB is encoded big-endian, and you didn't use a /bits/ 16 declaration
> (making the property a u32 cell), won't this read 0 in all cases you have a
> value in the expected range (as in your example)?
>
I will mention these values as 16bit. values in binding.
>
> > + of_property_read_u16(node, "ti,vendor",
> > + &input_dev->id.vendor);
> > + of_property_read_u16(node, "ti,product",
> > + &input_dev->id.product);
> > + of_property_read_u16(node, "ti,version",
> > + &input_dev->id.version);
>
> Similarly for these three.
>
Also, I will mention these values as 16bit in binding.
>
> Thanks,
> Mark.
>
Thanks,
Manish Badarkhe
[-- Attachment #1.2: Type: text/html, Size: 11889 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox