* [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
* Re: [PATCH V4 1/2] tps6507x-ts: Add DT support
From: Mark Rutland @ 2013-10-24 17:27 UTC (permalink / raw)
To: Manish Badarkhe
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: <CAKDJKT4+ma+KdNX0uemZUQVKHp8giGUX8EgWkY40vYOR-KR0_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thu, Oct 24, 2013 at 06:05:53PM +0100, Manish Badarkhe wrote:
> 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.
I was trying to say that nodes != properties rather than that this should be
split into separate files.
>
> 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?
Please elaborate. Why will the default period be sub-optimal? What's the
tradeoff here?
>
>
> > + 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?
Why is it a u16, it's very uncommon to have u16 properties.
What _physical_ units is this in, and what order of magnitude? e.g. Pascals,
millipascals.
>
>
> >
> > 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.
Why?
>
>
> What values make sense?
>
>
> These values are only to tell about chip details.
That does not describe the set of valid values.
Is ti,vendor = <4> valid? What does this mean?
Is there some standard for assignment of vendor IDs that this follows?
>
>
> 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?
No. As I said, it traverses the list of all nodes. Is there is no matching
child, it will go on and possibly match a node that is not a direct child (e.g.
a child of another node).
Perhaps of_get_child_by_name is what you want.
>
>
>
> > + 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.
Please explain _why_ they are 16-bit values. Even if they are 16-bit it may
make sense to have them as u32 values for general consistency and least
surprise.
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device
From: Uwe Kleine-König @ 2013-10-24 18:26 UTC (permalink / raw)
To: Joe Perches
Cc: linux-kernel, Kukjin Kim, Ferruh Yigit, Wan ZongShun,
Andrey Moiseev, Dmitry Torokhov, Josh Wu, linux-samsung-soc,
Henrik Rydberg, Ben Dooks, linux-input, Pau Oliva Fora,
linux-arm-kernel
In-Reply-To: <cc5e6a317bc9ef33cfdfae3ed9d3a47b5319a47b.1382555436.git.joe@perches.com>
Hello Joe,
On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
> Emitting an OOM message isn't necessary after input_allocate_device
> as there's a generic OOM and a dump_stack already done.
>
> [...]
> Signed-off-by: Joe Perches <joe@perches.com>
> diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
> index 005d852..3b9c709 100644
> --- a/drivers/input/joystick/as5011.c
> +++ b/drivers/input/joystick/as5011.c
> @@ -254,8 +254,6 @@ static int as5011_probe(struct i2c_client *client,
> as5011 = kmalloc(sizeof(struct as5011_device), GFP_KERNEL);
> input_dev = input_allocate_device();
> if (!as5011 || !input_dev) {
> - dev_err(&client->dev,
> - "Can't allocate memory for device structure\n");
Don't know if that can happen, but if as5011 is NULL but input_dev isn't
the message would still be sensible, wouldn't it? There are several more
that suffer the same "problem".
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device
From: Dmitry Torokhov @ 2013-10-24 18:37 UTC (permalink / raw)
To: Joe Perches
Cc: linux-kernel, Wan ZongShun, Andrey Moiseev, Henrik Rydberg,
Josh Wu, Ferruh Yigit, Pau Oliva Fora, Ben Dooks, Kukjin Kim,
linux-input, linux-arm-kernel, linux-samsung-soc
In-Reply-To: <cc5e6a317bc9ef33cfdfae3ed9d3a47b5319a47b.1382555436.git.joe@perches.com>
Hi Joe,
On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
> Emitting an OOM message isn't necessary after input_allocate_device
> as there's a generic OOM and a dump_stack already done.
No, please don't. The kzalloc may get changed in the future to not dump
stack (that was added originally because not everyone was handling OOM
properly, right?), input core might get changed to use something else
than kzalloc, etc, etc.
The majority of errors use dev_err so we also get idea what device
failed (if there are several), and more.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device
From: Joe Perches @ 2013-10-24 18:43 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-kernel, Kukjin Kim, Ferruh Yigit, Wan ZongShun,
Andrey Moiseev, Dmitry Torokhov, Josh Wu, linux-samsung-soc,
Henrik Rydberg, Ben Dooks, linux-input, Pau Oliva Fora,
linux-arm-kernel
In-Reply-To: <20131024182611.GA3512@pengutronix.de>
On Thu, 2013-10-24 at 20:26 +0200, Uwe Kleine-König wrote:
> Hello Joe,
>
> On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
> > Emitting an OOM message isn't necessary after input_allocate_device
> > as there's a generic OOM and a dump_stack already done.
> >
> > [...]
> > Signed-off-by: Joe Perches <joe@perches.com>
> > diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
> > index 005d852..3b9c709 100644
> > --- a/drivers/input/joystick/as5011.c
> > +++ b/drivers/input/joystick/as5011.c
> > @@ -254,8 +254,6 @@ static int as5011_probe(struct i2c_client *client,
> > as5011 = kmalloc(sizeof(struct as5011_device), GFP_KERNEL);
> > input_dev = input_allocate_device();
> > if (!as5011 || !input_dev) {
> > - dev_err(&client->dev,
> > - "Can't allocate memory for device structure\n");
> Don't know if that can happen, but if as5011 is NULL but input_dev isn't
> the message would still be sensible, wouldn't it? There are several more
> that suffer the same "problem".
Any k.alloc without __GFP_NOWARN does a generic OOM message
and a dump_stack() so there could already be 2 messages anyway.
cheers, Joe
--
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: [PATCH 4/8] input: Remove OOM message after input_allocate_device
From: Joe Perches @ 2013-10-24 18:45 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, Wan ZongShun, Andrey Moiseev, Henrik Rydberg,
Josh Wu, Ferruh Yigit, Pau Oliva Fora, Ben Dooks, Kukjin Kim,
linux-input, linux-arm-kernel, linux-samsung-soc
In-Reply-To: <20131024183707.GA5281@core.coreip.homeip.net>
On Thu, 2013-10-24 at 11:37 -0700, Dmitry Torokhov wrote:
> Hi Joe,
>
> On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
> > Emitting an OOM message isn't necessary after input_allocate_device
> > as there's a generic OOM and a dump_stack already done.
>
> No, please don't. The kzalloc may get changed in the future to not dump
> stack (that was added originally because not everyone was handling OOM
> properly, right?), input core might get changed to use something else
> than kzalloc, etc, etc.
>
> The majority of errors use dev_err so we also get idea what device
> failed (if there are several), and more.
I think that's not valuable as input_allocate_device already has
dozens of locations that don't emit a specific OOM and centralizing
the location for any generic message would work anyway.
^ permalink raw reply
* Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device
From: Uwe Kleine-König @ 2013-10-24 18:46 UTC (permalink / raw)
To: Joe Perches
Cc: Kukjin Kim, Ferruh Yigit, Wan ZongShun, Andrey Moiseev,
Dmitry Torokhov, linux-kernel, Josh Wu, linux-samsung-soc,
Henrik Rydberg, Ben Dooks, linux-input, Pau Oliva Fora,
linux-arm-kernel
In-Reply-To: <1382640218.22433.71.camel@joe-AO722>
On Thu, Oct 24, 2013 at 11:43:38AM -0700, Joe Perches wrote:
> On Thu, 2013-10-24 at 20:26 +0200, Uwe Kleine-König wrote:
> > Hello Joe,
> >
> > On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
> > > Emitting an OOM message isn't necessary after input_allocate_device
> > > as there's a generic OOM and a dump_stack already done.
> > >
> > > [...]
> > > Signed-off-by: Joe Perches <joe@perches.com>
> > > diff --git a/drivers/input/joystick/as5011.c b/drivers/input/joystick/as5011.c
> > > index 005d852..3b9c709 100644
> > > --- a/drivers/input/joystick/as5011.c
> > > +++ b/drivers/input/joystick/as5011.c
> > > @@ -254,8 +254,6 @@ static int as5011_probe(struct i2c_client *client,
> > > as5011 = kmalloc(sizeof(struct as5011_device), GFP_KERNEL);
> > > input_dev = input_allocate_device();
> > > if (!as5011 || !input_dev) {
> > > - dev_err(&client->dev,
> > > - "Can't allocate memory for device structure\n");
> > Don't know if that can happen, but if as5011 is NULL but input_dev isn't
> > the message would still be sensible, wouldn't it? There are several more
> > that suffer the same "problem".
>
> Any k.alloc without __GFP_NOWARN does a generic OOM message
> and a dump_stack() so there could already be 2 messages anyway.
Then mention that in the commit log if you still want this patch?!
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device
From: Joe Perches @ 2013-10-24 18:48 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Kukjin Kim, Ferruh Yigit, Wan ZongShun, Andrey Moiseev,
Dmitry Torokhov, linux-kernel, Josh Wu, linux-samsung-soc,
Henrik Rydberg, Ben Dooks, linux-input, Pau Oliva Fora,
linux-arm-kernel
In-Reply-To: <20131024184612.GC3512@pengutronix.de>
On Thu, 2013-10-24 at 20:46 +0200, Uwe Kleine-König wrote:
> On Thu, Oct 24, 2013 at 11:43:38AM -0700, Joe Perches wrote:
[]
> > Any k.alloc without __GFP_NOWARN does a generic OOM message
> > and a dump_stack() so there could already be 2 messages anyway.
> Then mention that in the commit log if you still want this patch?!
How do you think this commit message should be changed:
Emitting an OOM message isn't necessary after input_allocate_device
as there's a generic OOM and a dump_stack already done.
^ permalink raw reply
* Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device
From: Dmitry Torokhov @ 2013-10-24 19:10 UTC (permalink / raw)
To: Joe Perches
Cc: linux-kernel, Wan ZongShun, Andrey Moiseev, Henrik Rydberg,
Josh Wu, Ferruh Yigit, Pau Oliva Fora, Ben Dooks, Kukjin Kim,
linux-input, linux-arm-kernel, linux-samsung-soc
In-Reply-To: <1382640339.22433.73.camel@joe-AO722>
On Thu, Oct 24, 2013 at 11:45:39AM -0700, Joe Perches wrote:
> On Thu, 2013-10-24 at 11:37 -0700, Dmitry Torokhov wrote:
> > Hi Joe,
> >
> > On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
> > > Emitting an OOM message isn't necessary after input_allocate_device
> > > as there's a generic OOM and a dump_stack already done.
> >
> > No, please don't. The kzalloc may get changed in the future to not dump
> > stack (that was added originally because not everyone was handling OOM
> > properly, right?), input core might get changed to use something else
> > than kzalloc, etc, etc.
> >
> > The majority of errors use dev_err so we also get idea what device
> > failed (if there are several), and more.
>
> I think that's not valuable as input_allocate_device already has
> dozens of locations that don't emit a specific OOM and centralizing
> the location for any generic message would work anyway.
Not having diagnostic messages in some of the drivers is hardly a
justification to remove them from everywhere else.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device
From: Uwe Kleine-König @ 2013-10-24 19:15 UTC (permalink / raw)
To: Joe Perches
Cc: Kukjin Kim, Ferruh Yigit, Wan ZongShun, Andrey Moiseev,
Dmitry Torokhov, linux-kernel, Josh Wu, linux-samsung-soc,
Henrik Rydberg, Ben Dooks, linux-input, Pau Oliva Fora,
linux-arm-kernel
In-Reply-To: <1382640528.22433.75.camel@joe-AO722>
On Thu, Oct 24, 2013 at 11:48:48AM -0700, Joe Perches wrote:
> On Thu, 2013-10-24 at 20:46 +0200, Uwe Kleine-König wrote:
> > On Thu, Oct 24, 2013 at 11:43:38AM -0700, Joe Perches wrote:
> []
> > > Any k.alloc without __GFP_NOWARN does a generic OOM message
> > > and a dump_stack() so there could already be 2 messages anyway.
> > Then mention that in the commit log if you still want this patch?!
>
> How do you think this commit message should be changed:
>
> Emitting an OOM message isn't necessary after input_allocate_device
> as there's a generic OOM and a dump_stack already done.
after input_allocate_device and/or k.alloc ...
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 4/8] input: Remove OOM message after input_allocate_device
From: Joe Perches @ 2013-10-24 19:20 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-kernel, Wan ZongShun, Andrey Moiseev, Henrik Rydberg,
Josh Wu, Ferruh Yigit, Pau Oliva Fora, Ben Dooks, Kukjin Kim,
linux-input, linux-arm-kernel, linux-samsung-soc
In-Reply-To: <20131024191032.GA5553@core.coreip.homeip.net>
On Thu, 2013-10-24 at 12:10 -0700, Dmitry Torokhov wrote:
> On Thu, Oct 24, 2013 at 11:45:39AM -0700, Joe Perches wrote:
> > On Thu, 2013-10-24 at 11:37 -0700, Dmitry Torokhov wrote:
> > > Hi Joe,
> > >
> > > On Wed, Oct 23, 2013 at 12:14:50PM -0700, Joe Perches wrote:
> > > > Emitting an OOM message isn't necessary after input_allocate_device
> > > > as there's a generic OOM and a dump_stack already done.
> > >
> > > No, please don't. The kzalloc may get changed in the future to not dump
> > > stack (that was added originally because not everyone was handling OOM
> > > properly, right?), input core might get changed to use something else
> > > than kzalloc, etc, etc.
> > >
> > > The majority of errors use dev_err so we also get idea what device
> > > failed (if there are several), and more.
> >
> > I think that's not valuable as input_allocate_device already has
> > dozens of locations that don't emit a specific OOM and centralizing
> > the location for any generic message would work anyway.
>
> Not having diagnostic messages in some of the drivers is hardly a
> justification to remove them from everywhere else.
But standardization is.
If a diagnostic message is really desired
without the already existing dump_stack(),
it's a small matter of adding something like:
drivers/input/input.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index c044699..98570c2 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -1736,19 +1736,23 @@ struct input_dev *input_allocate_device(void)
{
struct input_dev *dev;
- dev = kzalloc(sizeof(struct input_dev), GFP_KERNEL);
- if (dev) {
- dev->dev.type = &input_dev_type;
- dev->dev.class = &input_class;
- device_initialize(&dev->dev);
- mutex_init(&dev->mutex);
- spin_lock_init(&dev->event_lock);
- INIT_LIST_HEAD(&dev->h_list);
- INIT_LIST_HEAD(&dev->node);
-
- __module_get(THIS_MODULE);
+ dev = kzalloc(sizeof(struct input_dev), GFP_KERNEL | __GFP_NOWARN);
+ if (!dev) {
+ pr_err("%pf: input_allocate_device failed\n",
+ __builtin_return_address(1));
+ return NULL;
}
+ dev->dev.type = &input_dev_type;
+ dev->dev.class = &input_class;
+ device_initialize(&dev->dev);
+ mutex_init(&dev->mutex);
+ spin_lock_init(&dev->event_lock);
+ INIT_LIST_HEAD(&dev->h_list);
+ INIT_LIST_HEAD(&dev->node);
+
+ __module_get(THIS_MODULE);
+
return dev;
}
EXPORT_SYMBOL(input_allocate_device);
^ permalink raw reply related
* Re: [PATCHv7][ 1/4] Input: tsc2007: Add device tree support.
From: Lothar Waßmann @ 2013-10-25 5:56 UTC (permalink / raw)
To: Grant Likely
Cc: Mark Rutland, devicetree, Dmitry Torokhov, Eric BXXnard,
Pawel Moll, Stephen Warren, Ian Campbell, Rob Herring,
Denis Carikli, Thierry Reding, Sascha Hauer, linux-input,
Shawn Guo, linux-arm-kernel
In-Reply-To: <20131024164746.3CB36C403B6@trevor.secretlab.ca>
Hi,
Grant Likely wrote:
> On Thu, 24 Oct 2013 14:42:13 +0200, Denis Carikli <denis@eukrea.com> wrote:
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: Pawel Moll <pawel.moll@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Stephen Warren <swarren@wwwdotorg.org>
> > Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> > Cc: devicetree@vger.kernel.org
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: linux-input@vger.kernel.org
> > Cc: Sascha Hauer <kernel@pengutronix.de>
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: Lothar Waßmann <LW@KARO-electronics.de>
> > Cc: Eric Bénard <eric@eukrea.com>
> > Signed-off-by: Denis Carikli <denis@eukrea.com>
> > ---
> > ChangeLog v6->v7:
> > - One small whitespace cleanup.
> > - The properties specific to that driver are now prefixed with "ti,".
> > - The ti,fuzzy property has now better documentation.
> > ---
> > .../bindings/input/touchscreen/tsc2007.txt | 45 +++++
> > drivers/input/touchscreen/tsc2007.c | 194 +++++++++++++++-----
> > 2 files changed, 198 insertions(+), 41 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> > new file mode 100644
> > index 0000000..516b63b
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> > @@ -0,0 +1,45 @@
> > +* Texas Instruments tsc2007 touchscreen controller
> > +
> > +Required properties:
> > +- compatible: must be "ti,tsc2007".
> > +- reg: I2C address of the chip.
> > +- ti,x-plate-ohms: X-plate resistance in ohms.
> > +
> > +Optional properties:
> > +- gpios: the interrupt gpio the chip is connected to (trough the penirq pin)
> > + (see GPIO binding[2] for more details).
>
> Hmmm, why is this needed? Is the line used for anything other than
> finding the irq number? If it is just an interrupt then an interrupts
> property is all that should be specified.
>
It's also the pendown detect GPIO. It asserts an interrupt when first
activated and is then polled in the driver until it goes inactive again.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v4] Input: add regulator haptic driver
From: Hyunhee Kim @ 2013-10-25 6:29 UTC (permalink / raw)
To: 'Mark Brown', dmitry.torokhov, peter.ujfalusi, wfp5p,
linux-input, linux-kernel, akpm
Cc: 'hyunhee.kim', kyungmin.park
In-Reply-To: <20131024133836.GG18506@sirena.org.uk>
From: Hyunhee Kim <hyunhee.kim@samsung.com>
Date: Wed, 9 Oct 2013 16:21:36 +0900
Subject: [PATCH] Input: add regulator haptic driver
Signed-off-by: Hyunhee Kim <hyunhee.kim@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Aristeu Rozanski <aris@ruivo.org>
---
drivers/input/misc/Kconfig | 10 ++
drivers/input/misc/Makefile | 1 +
drivers/input/misc/regulator-haptic.c | 187
+++++++++++++++++++++++++++++++++
3 files changed, 198 insertions(+)
create mode 100644 drivers/input/misc/regulator-haptic.c
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index bb698e1..21b4d5b 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -82,6 +82,16 @@ config INPUT_ARIZONA_HAPTICS
To compile this driver as a module, choose M here: the
module will be called arizona-haptics.
+config INPUT_REGULATOR_HAPTIC
+ tristate "support haptics on/off using regulator"
+ select INPUT_FF_MEMLESS
+ help
+ Say Y to enable support for the haptic module. Haptic can be
+ enabled/disabled by regulator.
+
+ To compile this driver as a module, choose M here: the
+ module will be called regulator-haptic.
+
config INPUT_BMA150
tristate "BMA150/SMB380 acceleration sensor support"
depends on I2C
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index d7fc17f..106f0bc 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_INPUT_ADXL34X_I2C) +=
adxl34x-i2c.o
obj-$(CONFIG_INPUT_ADXL34X_SPI) += adxl34x-spi.o
obj-$(CONFIG_INPUT_APANEL) += apanel.o
obj-$(CONFIG_INPUT_ARIZONA_HAPTICS) += arizona-haptics.o
+obj-$(CONFIG_INPUT_REGULATOR_HAPTIC) += regulator-haptic.o
obj-$(CONFIG_INPUT_ATI_REMOTE2) += ati_remote2.o
obj-$(CONFIG_INPUT_ATLAS_BTNS) += atlas_btns.o
obj-$(CONFIG_INPUT_BFIN_ROTARY) += bfin_rotary.o
diff --git a/drivers/input/misc/regulator-haptic.c
b/drivers/input/misc/regulator-haptic.c
new file mode 100644
index 0000000..bab2ad9
--- /dev/null
+++ b/drivers/input/misc/regulator-haptic.c
@@ -0,0 +1,187 @@
+/*
+ * Regulator haptic driver
+ *
+ * Copyright (c) 2013 Samsung Electronics Co., Ltd.
+ *
+ * Author: Hyunhee Kim <hyunhee.kim@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/input.h>
+#include <linux/slab.h>
+#include <linux/regulator/driver.h>
+
+struct regulator_haptic {
+ struct device *dev;
+ struct input_dev *input_dev;
+ struct work_struct work;
+ bool enabled;
+ struct regulator *regulator;
+ struct mutex mutex;
+ int level;
+};
+
+static void regulator_haptic_enable(struct regulator_haptic *haptic)
+{
+ int ret;
+
+ if (!haptic->enabled) {
+ ret = regulator_enable(haptic->regulator);
+ if (ret)
+ dev_err(haptic->dev, "failed to enable
regulator\n");
+ haptic->enabled = true;
+ }
+}
+
+static void regulator_haptic_disable(struct regulator_haptic *haptic)
+{
+ int ret;
+
+ if (haptic->enabled) {
+ ret = regulator_disable(haptic->regulator);
+ if (ret)
+ dev_err(haptic->dev, "failed to disable
regulator\n");
+ haptic->enabled = false;
+ }
+}
+
+static void regulator_haptic_work(struct work_struct *work)
+{
+ struct regulator_haptic *haptic = container_of(work,
+ struct
regulator_haptic,
+ work);
+ mutex_lock(&haptic->mutex);
+ if (haptic->level)
+ regulator_haptic_enable(haptic);
+ else
+ regulator_haptic_disable(haptic);
+ mutex_unlock(&haptic->mutex);
+}
+
+static int regulator_haptic_play(struct input_dev *input, void *data,
+ struct ff_effect *effect)
+{
+ struct regulator_haptic *haptic = input_get_drvdata(input);
+
+ haptic->level = effect->u.rumble.strong_magnitude;
+ if (!haptic->level)
+ haptic->level = effect->u.rumble.weak_magnitude;
+ schedule_work(&haptic->work);
+
+ return 0;
+}
+
+static void regulator_haptic_close(struct input_dev *input)
+{
+ struct regulator_haptic *haptic = input_get_drvdata(input);
+
+ cancel_work_sync(&haptic->work);
+ regulator_haptic_disable(haptic);
+}
+
+static int regulator_haptic_probe(struct platform_device *pdev)
+{
+ struct regulator_haptic *haptic;
+ struct input_dev *input_dev;
+ int error;
+
+ haptic = devm_kzalloc(&pdev->dev, sizeof(*haptic), GFP_KERNEL);
+ if (!haptic) {
+ dev_err(&pdev->dev, "unable to allocate memory for
haptic\n");
+ return -ENOMEM;
+ }
+
+ input_dev = input_allocate_device();
+
+ if (!input_dev) {
+ dev_err(&pdev->dev, "unable to allocate memory\n");
+ return -ENOMEM;
+ }
+
+ INIT_WORK(&haptic->work, regulator_haptic_work);
+ mutex_init(&haptic->mutex);
+ haptic->input_dev = input_dev;
+ haptic->dev = &pdev->dev;
+ haptic->regulator = regulator_get(&pdev->dev, "haptic");
+
+ if (IS_ERR(haptic->regulator)) {
+ error = PTR_ERR(haptic->regulator);
+ dev_err(&pdev->dev, "unable to get regulator, err: %d\n",
+ error);
+ goto err_ifree_mem;
+ }
+
+ haptic->input_dev->name = "regulator:haptic";
+ haptic->input_dev->dev.parent = &pdev->dev;
+ haptic->input_dev->close = regulator_haptic_close;
+ haptic->enabled = false;
+ input_set_drvdata(haptic->input_dev, haptic);
+ input_set_capability(haptic->input_dev, EV_FF, FF_RUMBLE);
+
+ error = input_ff_create_memless(input_dev, NULL,
+ regulator_haptic_play);
+ if (error) {
+ dev_err(&pdev->dev,
+ "input_ff_create_memless() failed: %d\n",
+ error);
+ goto err_put_regulator;
+ }
+
+ error = input_register_device(haptic->input_dev);
+ if (error) {
+ dev_err(&pdev->dev,
+ "couldn't register input device: %d\n",
+ error);
+ goto err_destroy_ff;
+ }
+
+ platform_set_drvdata(pdev, haptic);
+
+ return 0;
+
+err_destroy_ff:
+ input_ff_destroy(haptic->input_dev);
+err_put_regulator:
+ regulator_put(haptic->regulator);
+err_ifree_mem:
+ input_free_device(haptic->input_dev);
+
+ return error;
+}
+
+static int regulator_haptic_remove(struct platform_device *pdev)
+{
+ struct regulator_haptic *haptic = platform_get_drvdata(pdev);
+
+ input_unregister_device(haptic->input_dev);
+ regulator_put(haptic->regulator);
+
+ return 0;
+}
+
+static struct of_device_id regulator_haptic_dt_match[] = {
+ { .compatible = "linux,regulator-haptic" },
+ {},
+};
+
+static struct platform_driver regulator_haptic_driver = {
+ .driver = {
+ .name = "regulator-haptic",
+ .owner = THIS_MODULE,
+ .of_match_table = regulator_haptic_dt_match,
+ },
+
+ .probe = regulator_haptic_probe,
+ .remove = regulator_haptic_remove,
+};
+module_platform_driver(regulator_haptic_driver);
+
+MODULE_ALIAS("platform:regulator-haptic");
+MODULE_DESCRIPTION("Regulator haptic driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Hyunhee Kim <hyunhee.kim@samsung.com>");
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCHv7][ 1/4] Input: tsc2007: Add device tree support.
From: Lothar Waßmann @ 2013-10-25 6:39 UTC (permalink / raw)
To: Thierry Reding
Cc: Mark Rutland, devicetree, Dmitry Torokhov, Eric Bénard,
Pawel Moll, Stephen Warren, Linus Walleij, Ian Campbell,
Rob Herring, Grant Likely, Denis Carikli, Sascha Hauer,
linux-input, Shawn Guo, linux-arm-kernel
In-Reply-To: <20131024151736.GE9044@ulmo.nvidia.com>
Hi,
Thierry Reding wrote:
> 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.
>
When a GPIO is requested the GPIO layer cannot know what it is
requested for and how the pinmux for the pin function should be
configured (pullups, drive strength, ...). A pin may have multiple
functions depending on what modules are plugged into a baseboard.
Thus the pinctrl binding needs to be with the driver that uses the pin.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
_______________________________________________
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] Input: add regulator haptic driver
From: Mark Brown @ 2013-10-25 8:29 UTC (permalink / raw)
To: Hyunhee Kim
Cc: dmitry.torokhov, peter.ujfalusi, wfp5p, linux-input, linux-kernel,
akpm, kyungmin.park
In-Reply-To: <002601ced14b$9c412330$d4c36990$%kim@samsung.com>
[-- Attachment #1: Type: text/plain, Size: 492 bytes --]
On Fri, Oct 25, 2013 at 03:29:51PM +0900, Hyunhee Kim wrote:
> From: Hyunhee Kim <hyunhee.kim@samsung.com>
> Date: Wed, 9 Oct 2013 16:21:36 +0900
> Subject: [PATCH] Input: add regulator haptic driver
This is OK from a regulator point of view though there's still a lot of
use of regulator devm_ and input devm_ that could happen - it wasn't
just the kzalloc(), it was most of the allocations in the driver.
You also have some word wrapping in your e-mail so the patch probably
won't apply.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCHv7][ 1/4] Input: tsc2007: Add device tree support.
From: Thierry Reding @ 2013-10-25 8:33 UTC (permalink / raw)
To: Lothar Waßmann
Cc: Denis Carikli, 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, Linus Walleij
In-Reply-To: <20131025083951.18675574@ipc1.ka-ro>
[-- Attachment #1: Type: text/plain, Size: 3967 bytes --]
On Fri, Oct 25, 2013 at 08:39:51AM +0200, Lothar Waßmann wrote:
> Hi,
>
> Thierry Reding wrote:
> > 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.
> >
> When a GPIO is requested the GPIO layer cannot know what it is
> requested for and how the pinmux for the pin function should be
> configured (pullups, drive strength, ...). A pin may have multiple
> functions depending on what modules are plugged into a baseboard.
But GPIO is one specific function of a given pin, isn't it?
> Thus the pinctrl binding needs to be with the driver that uses the pin.
I wonder if this really needs to be dynamically configured depending on
the driver. Properties like pullups and driver strength can potentially
harm hardware when they're wrong, so relying on the driver's probe to
configure them correctly doesn't sound very safe. It's always possible
to disable the device (either via modifying the DTS or by having the
bootloader patch it). That means the driver won't be probed at all, and
therefore none of the pinctrl or pinmux settings will be applied.
Thierry
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] HID: hid-multitouch: add support for SiS panels
From: Jiri Kosina @ 2013-10-25 8:47 UTC (permalink / raw)
To: Forest Bond; +Cc: linux-input
In-Reply-To: <20131021163849.GC30240@alittletooquiet.net>
On Mon, 21 Oct 2013, Forest Bond wrote:
> From: Forest Bond <forest.bond@rapidrollout.com>
>
> Signed-off-by: Forest Bond <forest.bond@rapidrollout.com>
Added changelog (I don't like commits without changelog) and applied.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: hid-sensor-hub: fix report size
From: Jiri Kosina @ 2013-10-25 9:01 UTC (permalink / raw)
To: Srinivas Pandruvada; +Cc: linux-input
In-Reply-To: <1382114924-25654-1-git-send-email-srinivas.pandruvada@linux.intel.com>
On Fri, 18 Oct 2013, Srinivas Pandruvada wrote:
> The number of bytes in a field needs to take account of report_count
> field. Need to multiply report_size and report_count to get total
> number of bytes.
Srinivas,
could you please elaborate a little bit more in the changelog what
user-visible bug this is fixing? Thanks.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
> drivers/hid/hid-sensor-hub.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 88fc5ae..a184e19 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -326,7 +326,8 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
> field->logical == attr_usage_id) {
> sensor_hub_fill_attr_info(info, i, report->id,
> field->unit, field->unit_exponent,
> - field->report_size);
> + field->report_size *
> + field->report_count);
> ret = 0;
> } else {
> for (j = 0; j < field->maxusage; ++j) {
> @@ -338,7 +339,8 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
> i, report->id,
> field->unit,
> field->unit_exponent,
> - field->report_size);
> + field->report_size *
> + field->report_count);
> ret = 0;
> break;
> }
> @@ -425,9 +427,10 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
> hid_dbg(hdev, "%d collection_index:%x hid:%x sz:%x\n",
> i, report->field[i]->usage->collection_index,
> report->field[i]->usage->hid,
> - report->field[i]->report_size/8);
> -
> - sz = report->field[i]->report_size/8;
> + (report->field[i]->report_size *
> + report->field[i]->report_count)/8);
> + sz = (report->field[i]->report_size *
> + report->field[i]->report_count)/8;
> if (pdata->pending.status && pdata->pending.attr_usage_id ==
> report->field[i]->usage->hid) {
> hid_dbg(hdev, "data was pending ...\n");
> --
> 1.8.3.2
>
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: Added Holtek USB ID 04d9:a072 LEETGION Hellion Gaming Mouse
From: Jiri Kosina @ 2013-10-25 9:28 UTC (permalink / raw)
To: Anders F. U. Kiær; +Cc: linux-input, linux-kernel
In-Reply-To: <1382391742-31335-1-git-send-email-ablacksheep@gmail.com>
On Mon, 21 Oct 2013, Anders F. U. Kiær wrote:
> Added id, bindings and comments for Holtek USB ID 04d9:a072
> LEETGION Hellion Gaming mouse to use the same corrections of the report
> descriptor as Holtek 04d9:a067. As the mouse exceed HID_MAX_USAGES at the
> same offsets in the reported descriptor.
> Tested on the hardware.
>
> Signed-off-by: Anders F. U. Kiær <ablacksheep@gmail.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: hid-multitouch: add support for SiS panels
From: Forest Bond @ 2013-10-25 11:56 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input
In-Reply-To: <alpine.LNX.2.00.1310250947340.30926@pobox.suse.cz>
[-- Attachment #1: Type: text/plain, Size: 427 bytes --]
Hi Jiri,
On Fri, Oct 25, 2013 at 09:47:55AM +0100, Jiri Kosina wrote:
> On Mon, 21 Oct 2013, Forest Bond wrote:
>
> > From: Forest Bond <forest.bond@rapidrollout.com>
> >
> > Signed-off-by: Forest Bond <forest.bond@rapidrollout.com>
>
> Added changelog (I don't like commits without changelog) and applied.
Noted. Thanks!
-Forest
--
Forest Bond
http://www.forestbond.com/
http://www.rapidrollout.com/
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 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