* Re: [PATCH v2] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
From: Dmitry Torokhov @ 2014-07-28 7:10 UTC (permalink / raw)
To: pramod gurav
Cc: linux-input, linux-kernel@vger.kernel.org, Pramod Gurav,
Lejun Zhu, Sachin Kamat
In-Reply-To: <CAMf-jSm1d0wAJ3C_yFhqnoDPSLqeDcc-ow0D=bwmLUcC9sXjLA@mail.gmail.com>
On July 27, 2014 11:50:41 PM PDT, pramod gurav <pramod.gurav.etc@gmail.com> wrote:
>Hi Dmitry,
>
>Thanks for the review.
>
>On Fri, Jul 25, 2014 at 9:52 PM, Dmitry Torokhov
><dmitry.torokhov@gmail.com> wrote:
>> Hi Pramod,
>>
>> On Fri, Jul 25, 2014 at 05:04:34PM +0530, pramod.gurav.etc@gmail.com
>wrote:
>>> From: Pramod Gurav <pramod.gurav@smartplayin.com>
>>>
>>> This patch does below:
>>> - Removes kfree done on data allocated with devm_zalloc in probe
>>> path of the driver.
>>> - Adds a check on return value from devm_kzalloc which was missing
>>>
>>> CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>> CC: Lejun Zhu <lejun.zhu@linux.intel.com>
>>> CC: Sachin Kamat <sachin.kamat@linaro.org>
>>>
>>> Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
>>> ---
>>> drivers/input/misc/soc_button_array.c | 17 +++++++----------
>>> 1 file changed, 7 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/input/misc/soc_button_array.c
>b/drivers/input/misc/soc_button_array.c
>>> index 5a6334b..fc64ec6 100644
>>> --- a/drivers/input/misc/soc_button_array.c
>>> +++ b/drivers/input/misc/soc_button_array.c
>>> @@ -83,6 +83,9 @@ soc_button_device_create(struct pnp_dev *pdev,
>>> sizeof(*gpio_keys_pdata) +
>>> sizeof(*gpio_keys) *
>MAX_NBUTTONS,
>>> GFP_KERNEL);
>>> + if (!gpio_keys_pdata)
>>> + return ERR_PTR(-ENOMEM);
>>
>> OK, that makes sense.
>>
>>> +
>>> gpio_keys = (void *)(gpio_keys_pdata + 1);
>>>
>>> for (info = button_info; info->name; info++) {
>>> @@ -102,20 +105,16 @@ soc_button_device_create(struct pnp_dev *pdev,
>>> n_buttons++;
>>> }
>>>
>>> - if (n_buttons == 0) {
>>> - error = -ENODEV;
>>> - goto err_free_mem;
>>> - }
>>> + if (n_buttons == 0)
>>> + return ERR_PTR(-ENODEV);
>>
>> But that one and the rest don't, because failure in
>> soc_button_device_create() does not necessarily mean that binding for
>> the whole device will fail. In this case we do not want unused memory
>> hang around.
>Agree. Should resend the patch with only the error check after mem
>allocation and will be little more careful while sending any such
>change. :)
No need to resend, I picked out the good bits and applied.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] Input: soc_button_array: Remove kfree on data allocated with devm_zalloc
From: pramod gurav @ 2014-07-28 7:24 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel@vger.kernel.org, Pramod Gurav,
Lejun Zhu, Sachin Kamat
In-Reply-To: <9c56a845-2005-49c5-90c3-1d22a89fad20@email.android.com>
On Mon, Jul 28, 2014 at 12:40 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On July 27, 2014 11:50:41 PM PDT, pramod gurav <pramod.gurav.etc@gmail.com> wrote:
>>Hi Dmitry,
>>
>
> No need to resend, I picked out the good bits and applied.
Thanks. :)
--
Thanks and Regards
Pramod
^ permalink raw reply
* Re: [PATCH] hid: uhid.c: Cleaning up missing null-terminate in conjunction with strncpy
From: David Herrmann @ 2014-07-28 8:31 UTC (permalink / raw)
To: Rickard Strandqvist
Cc: David Herrmann, Jiri Kosina, open list:HID CORE LAYER,
linux-kernel
In-Reply-To: <1406393198-5306-1-git-send-email-rickard_strandqvist@spectrumdigital.se>
Hi
On Sat, Jul 26, 2014 at 6:46 PM, Rickard Strandqvist
<rickard_strandqvist@spectrumdigital.se> wrote:
> Replacing strncpy with strlcpy to avoid strings that lacks null terminate.
> And removed unnecessary magic numbers.
>
> Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
> ---
> drivers/hid/uhid.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 0cb92e3..b72ab0c 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -392,12 +392,9 @@ static int uhid_dev_create(struct uhid_device *uhid,
> goto err_free;
> }
>
> - strncpy(hid->name, ev->u.create.name, 127);
> - hid->name[127] = 0;
> - strncpy(hid->phys, ev->u.create.phys, 63);
> - hid->phys[63] = 0;
> - strncpy(hid->uniq, ev->u.create.uniq, 63);
> - hid->uniq[63] = 0;
> + strlcpy(hid->name, ev->u.create.name, sizeof(hid->name));
> + strlcpy(hid->phys, ev->u.create.phys, sizeof(hid->phys));
> + strlcpy(hid->uniq, ev->u.create.uniq, sizeof(hid->uniq));
NAK, this patch is wrong. strlcpy() is *not* a replacement for
strncpy(). Please see lib/string.c or the man-page of strlcpy(3). It
calls strlen() on the source, which strncpy() does not. "ev" was
copied unmodified from user-space and might not be NULL-terminated,
therefore, strncpy() is the right choice.
Btw., please avoid commit-messages that sound like bug-fixes, which
this definitely is not. hid_allocate_device() cleans the object to
ZERO, therefore, you're guaranteed that there's a proper
NULL-terminator, independent of the contents of ev->u.xyz.
Thanks
David
> hid->ll_driver = &uhid_hid_driver;
> hid->bus = ev->u.create.bus;
> @@ -452,12 +449,9 @@ static int uhid_dev_create2(struct uhid_device *uhid,
> goto err_free;
> }
>
> - strncpy(hid->name, ev->u.create2.name, 127);
> - hid->name[127] = 0;
> - strncpy(hid->phys, ev->u.create2.phys, 63);
> - hid->phys[63] = 0;
> - strncpy(hid->uniq, ev->u.create2.uniq, 63);
> - hid->uniq[63] = 0;
> + strlcpy(hid->name, ev->u.create2.name, sizeof(hid->name));
> + strlcpy(hid->phys, ev->u.create2.phys, sizeof(hid->phys));
> + strlcpy(hid->uniq, ev->u.create2.uniq, sizeof(hid->uniq));
>
> hid->ll_driver = &uhid_hid_driver;
> hid->bus = ev->u.create2.bus;
> --
> 1.7.10.4
>
^ permalink raw reply
* [PATCH] input: ads7846: Switch to managed version of kzalloc and cleanups
From: pramod.gurav.etc @ 2014-07-28 8:46 UTC (permalink / raw)
To: linux-input, linux-kernel
Cc: Pramod Gurav, Dmitry Torokhov, Guenter Roeck, Paul Gortmaker,
Mark Brown, Fabio Estevam
From: Pramod Gurav <pramod.gurav@smartplayin.com>
This switches memory allocations from kzalloc to devm_kzalloc.
This also changes the way return checks were done on failure cases
of three allocations. The checks were clubbed together and hence
must be done seperately to avoid calling kfree on unallocated memory.
Moreover in case of input_allocate_device failure, we were calling
input_free_device which is not needed.
input device must be released(input_free_device) when ads7846_probe_dt
fails hence adds a fix there as well.
CC: Dmitry Torokhov <dmitry.torokhov@gmail.com>
CC: Guenter Roeck <linux@roeck-us.net>
CC: Paul Gortmaker <paul.gortmaker@windriver.com>
CC: Mark Brown <broonie@linaro.org>
CC: Fabio Estevam <fabio.estevam@freescale.com>
Signed-off-by: Pramod Gurav <pramod.gurav@smartplayin.com>
---
Earlier when kzalloc was used to allocate, the error check for all three
resources (ts, packet, input_dev) was done together. This I felt might
cause a problem in case any of these allocations fails and we may end up
calling kfree on unallocated memory. Moving to managed resource solves this
anyway as we remove references of kfree in probe fail path.
Moving to managed resources solves this anyway.
drivers/input/touchscreen/ads7846.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index da201b8..d229b8d 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -1282,13 +1282,17 @@ static int ads7846_probe(struct spi_device *spi)
if (err < 0)
return err;
- ts = kzalloc(sizeof(struct ads7846), GFP_KERNEL);
- packet = kzalloc(sizeof(struct ads7846_packet), GFP_KERNEL);
+ ts = devm_kzalloc(&spi->dev, sizeof(*ts), GFP_KERNEL);
+ if (!ts)
+ return -ENOMEM;
+
+ packet = devm_kzalloc(&spi->dev, sizeof(*packet), GFP_KERNEL);
+ if (!packet)
+ return -ENOMEM;
+
input_dev = input_allocate_device();
- if (!ts || !packet || !input_dev) {
- err = -ENOMEM;
- goto err_free_mem;
- }
+ if (!input_dev)
+ return -ENOMEM;
spi_set_drvdata(spi, ts);
@@ -1302,8 +1306,10 @@ static int ads7846_probe(struct spi_device *spi)
pdata = dev_get_platdata(&spi->dev);
if (!pdata) {
pdata = ads7846_probe_dt(&spi->dev);
- if (IS_ERR(pdata))
- return PTR_ERR(pdata);
+ if (IS_ERR(pdata)) {
+ err = PTR_ERR(pdata);
+ goto err_free_mem;
+ }
}
ts->model = pdata->model ? : 7846;
@@ -1450,8 +1456,6 @@ static int ads7846_probe(struct spi_device *spi)
ts->filter_cleanup(ts->filter_data);
err_free_mem:
input_free_device(input_dev);
- kfree(packet);
- kfree(ts);
return err;
}
@@ -1484,9 +1488,6 @@ static int ads7846_remove(struct spi_device *spi)
if (ts->filter_cleanup)
ts->filter_cleanup(ts->filter_data);
- kfree(ts->packet);
- kfree(ts);
-
dev_dbg(&spi->dev, "unregistered touchscreen\n");
return 0;
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v7 0/5] Input: pixcir_i2c_ts: Add Type-B Multi-touch and DT support
From: Roger Quadros @ 2014-07-28 9:16 UTC (permalink / raw)
To: dmitry.torokhov, tony
Cc: rydberg, balbi, dmurphy, mugunthanvnm, nsekhar, linux-input,
linux-kernel, linux-omap
In-Reply-To: <53B523AE.70601@ti.com>
Hi Dmitry,
Gentle ping. Would be nice to get these into 3.17. Thanks :).
cheers,
-roger
On 07/03/2014 12:34 PM, Roger Quadros wrote:
> Hi Dmitry,
>
> Gentle reminder to pick this series for -next. Thanks :).
>
> cheers,
> -roger
>
> On 06/17/2014 12:31 PM, Roger Quadros wrote:
>> Hi Dmitry,
>>
>> These are the pending patches that didn't go through in the 3.16 merge window.
>> Please queue them for -next. Thanks.
>>
>> The series does the following
>> - convert to Type-B multi touch protocol
>> - support upto 5 fingers with hardware supplied tracking IDs
>> - device tree support
>>
>> Tony,
>>
>> The last 2 patches fix the touch screen size property in the device tree since
>> we now use the unified touch screen properties.
>>
>> cheers,
>> -roger
>>
>> Changelog:
>>
>> v7:
>> - Rebased on 3.16-rc1.
>>
>> v6:
>> - Use unified touchscreen bindings.
>> - Update pointer math i.e. bufptr += 4 instead of &bufptr[4];
>>
>> v5:
>> - Changed ts->exiting flag to ts->running in patch 2.
>> - Don't call pixcir_stop() from .suspend() if wakeup is required.
>>
>> v4:
>> - Imporved pixcir_stop() as per Dmitry's suggestion.
>> - Removed unnecessary input_unregister_device() from .remove().
>>
>> v3:
>> - Rebased to 3.15-rc3
>> - Fixed suspend while touchscreen in use
>> - Fixed module removal while touchscreen in use
>>
>> v2:
>> - Addressed review comments and re-arranged patch order
>>
>> v1:
>> - http://article.gmane.org/gmane.linux.kernel/1616417
>>
>> --
>>
>> Roger Quadros (5):
>> Input: pixcir_i2c_ts: Use Type-B Multi-Touch protocol
>> Input: pixcir_i2c_ts: support upto 5 fingers and hardware provided
>> tracking IDs
>> Input: pixcir_i2c_ts: Add device tree support
>> ARM: dts: am43x-epos-evm: Update binding for touchscreen size
>> ARM: dts: am437x-gp-evm: Update binding for touchscreen size
>>
>> .../bindings/input/touchscreen/pixcir_i2c_ts.txt | 26 +++
>> .../devicetree/bindings/vendor-prefixes.txt | 1 +
>> arch/arm/boot/dts/am437x-gp-evm.dts | 4 +-
>> arch/arm/boot/dts/am43x-epos-evm.dts | 4 +-
>> drivers/input/touchscreen/pixcir_i2c_ts.c | 251 ++++++++++++++++++---
>> include/linux/input/pixcir_ts.h | 12 +
>> 6 files changed, 259 insertions(+), 39 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/pixcir_i2c_ts.txt
>>
>
^ permalink raw reply
* [PATCH 0/7] Second batch of cleanups for cros_ec
From: Javier Martinez Canillas @ 2014-07-28 12:19 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
Olof Johansson, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
Javier Martinez Canillas
This is a second batch of cleanups patches for the mfd cros_ec
driver and its subdevices drivers. The first batch of cleanups
was posted by Doug Anderson [0] and have already been merged.
The patches were picked from the ChromeOS 3.8 kernel and after
these no cleanups patches for cros_ec are left, only commits
that add cros ec support not yet available in mainline.
There is almost no functionality added on this series but the
idea is to reduce the delta between the mainline drivers and
the ones in the downstream Chrome OS 3.8 kernel so the missing
functionality can be added on top once these cleanups patches
are merged. The missing functionlity currently in mainline is:
- Chrome OS Embedded Controller userspace device interface
- Chrome OS Embedded Controller Low Pin Count (LPC) inteface
- Access to vboot context stored on a block device
- Access to vboot context stored on EC's nvram
The patches in this series are authored by different people
(all on cc) and consist of the following:
Andrew Bresticker (3):
mfd: cros_ec: stop calling ->cmd_xfer() directly
mfd: cros_ec: move locking into cros_ec_cmd_xfer
mfd: cros_ec: wait for completion of commands that return IN_PROGRESS
Derek Basehore (1):
i2c: i2c-cros-ec-tunnel: Set retries to 3
Doug Anderson (1):
mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC
Todd Broch (2):
mfd: cros_ec: Instantiate sub-devices from device tree
Input: cros_ec_keyb: Optimize ghosting algorithm.
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 5 +-
drivers/input/keyboard/cros_ec_keyb.c | 89 +++++++++++++++++----------------
drivers/mfd/cros_ec.c | 88 ++++++++++++++++++++++++++++----
drivers/mfd/cros_ec_spi.c | 20 ++++----
include/linux/mfd/cros_ec.h | 24 ++++++---
5 files changed, 154 insertions(+), 72 deletions(-)
The patches should be merged together which means that some of
them need to be acked by relevant maintainers like patch 02/07
by the i2c maintainer and patch 07/07 by the input maintainer.
Best regards,
Javier
[0]: https://lkml.org/lkml/2014/6/16/681
^ permalink raw reply
* [PATCH 1/7] mfd: cros_ec: Delay for 50ms when we see EC_CMD_REBOOT_EC
From: Javier Martinez Canillas @ 2014-07-28 12:19 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
Olof Johansson, linux-i2c, linux-input, linux-samsung-soc
In-Reply-To: <1406549967-21291-1-git-send-email-javier.martinez@collabora.co.uk>
From: Doug Anderson <dianders@chromium.org>
If someone sends a EC_CMD_REBOOT_EC to the EC, the EC will likely be
unresponsive for quite a while. Add a delay to the end of the command
to prevent random failures of future commands.
NOTES:
* This could be optimized a bit by simply delaying the next command
sent, but EC_CMD_REBOOT_EC is such a rare command that the extra
complexity doesn't seem worth it.
* This is a bit of an "ugly hack" since the SPI driver is effectively
snooping on the communication and making a lot of assumptions. It
would be nice to architect in some better solution long term.
* This same logic probably needs to be applied to the i2c driver.
Signed-off-by: Doug Anderson <dianders@chromium.org>
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
---
drivers/mfd/cros_ec_spi.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index ac52e36..6726f0a 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -65,6 +65,12 @@
*/
#define EC_SPI_RECOVERY_TIME_NS (200 * 1000)
+/*
+ * The EC is unresponsive for a time after a reboot command. Add a
+ * simple delay to make sure that the bus stays locked.
+ */
+#define EC_REBOOT_DELAY_MS 50
+
/**
* struct cros_ec_spi - information about a SPI-connected EC
*
@@ -322,6 +328,9 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
ret = len;
exit:
+ if (ec_msg->command == EC_CMD_REBOOT_EC)
+ msleep(EC_REBOOT_DELAY_MS);
+
mutex_unlock(&ec_spi->lock);
return ret;
}
--
2.0.0.rc2
^ permalink raw reply related
* [PATCH 2/7] i2c: i2c-cros-ec-tunnel: Set retries to 3
From: Javier Martinez Canillas @ 2014-07-28 12:19 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
Olof Johansson, linux-i2c, linux-input, linux-samsung-soc
In-Reply-To: <1406549967-21291-1-git-send-email-javier.martinez@collabora.co.uk>
From: Derek Basehore <dbasehore@chromium.org>
Since the i2c bus can get wedged on the EC sometimes, set the number of retries
to 3. Since we un-wedge the bus immediately after the wedge happens, this is the
correct fix since only one transfer will fail.
Signed-off-by: Derek Basehore <dbasehore@chromium.org>
Reviewed-by: Doug Anderson <dianders@chromium.org>
---
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index 05e033c..a4411da 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -16,6 +16,8 @@
#include <linux/platform_device.h>
#include <linux/slab.h>
+#define I2C_MAX_RETRIES 3
+
/**
* struct ec_i2c_device - Driver data for I2C tunnel
*
@@ -290,6 +292,7 @@ static int ec_i2c_probe(struct platform_device *pdev)
bus->adap.algo_data = bus;
bus->adap.dev.parent = &pdev->dev;
bus->adap.dev.of_node = np;
+ bus->adap.retries = I2C_MAX_RETRIES;
err = i2c_add_adapter(&bus->adap);
if (err) {
--
2.0.0.rc2
^ permalink raw reply related
* [PATCH 3/7] mfd: cros_ec: stop calling ->cmd_xfer() directly
From: Javier Martinez Canillas @ 2014-07-28 12:19 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
Olof Johansson, linux-i2c, linux-input, linux-samsung-soc
In-Reply-To: <1406549967-21291-1-git-send-email-javier.martinez@collabora.co.uk>
From: Andrew Bresticker <abrestic@chromium.org>
Instead of having users of the ChromeOS EC call the interface-specific
cmd_xfer() callback directly, introduce a central cros_ec_cmd_xfer()
to use instead. This will allow us to put all the locking and retry
logic in one place instead of duplicating it across the different
drivers.
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
drivers/i2c/busses/i2c-cros-ec-tunnel.c | 2 +-
drivers/input/keyboard/cros_ec_keyb.c | 2 +-
drivers/mfd/cros_ec.c | 7 +++++++
include/linux/mfd/cros_ec.h | 24 ++++++++++++++++++------
4 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cros-ec-tunnel.c b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
index a4411da..8ca5cbb 100644
--- a/drivers/i2c/busses/i2c-cros-ec-tunnel.c
+++ b/drivers/i2c/busses/i2c-cros-ec-tunnel.c
@@ -229,7 +229,7 @@ static int ec_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg i2c_msgs[],
msg.indata = response;
msg.insize = response_len;
- result = bus->ec->cmd_xfer(bus->ec, &msg);
+ result = cros_ec_cmd_xfer(bus->ec, &msg);
if (result < 0)
goto exit;
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 791781a..93111d1 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -182,7 +182,7 @@ static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
.insize = ckdev->cols,
};
- return ckdev->ec->cmd_xfer(ckdev->ec, &msg);
+ return cros_ec_cmd_xfer(ckdev->ec, &msg);
}
static irqreturn_t cros_ec_keyb_irq(int irq, void *data)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 4873f9c..a9faebd 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -62,6 +62,13 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
}
EXPORT_SYMBOL(cros_ec_check_result);
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg)
+{
+ return ec_dev->cmd_xfer(ec_dev, msg);
+}
+EXPORT_SYMBOL(cros_ec_cmd_xfer);
+
static const struct mfd_cell cros_devs[] = {
{
.name = "cros-ec-keyb",
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index fcbe9d1..0e166b9 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -62,10 +62,6 @@ struct cros_ec_command {
* @dev: Device pointer
* @was_wake_device: true if this device was set to wake the system from
* sleep at the last suspend
- * @cmd_xfer: send command to EC and get response
- * Returns the number of bytes received if the communication succeeded, but
- * that doesn't mean the EC was happy with the command. The caller
- * should check msg.result for the EC's result code.
*
* @priv: Private data
* @irq: Interrupt to use
@@ -82,6 +78,10 @@ struct cros_ec_command {
* @dout_size: size of dout buffer to allocate (zero to use static dout)
* @parent: pointer to parent device (e.g. i2c or spi device)
* @wake_enabled: true if this device can wake the system from sleep
+ * @cmd_xfer: send command to EC and get response
+ * Returns the number of bytes received if the communication succeeded, but
+ * that doesn't mean the EC was happy with the command. The caller
+ * should check msg.result for the EC's result code.
* @lock: one transaction at a time
*/
struct cros_ec_device {
@@ -92,8 +92,6 @@ struct cros_ec_device {
struct device *dev;
bool was_wake_device;
struct class *cros_class;
- int (*cmd_xfer)(struct cros_ec_device *ec,
- struct cros_ec_command *msg);
/* These are used to implement the platform-specific interface */
void *priv;
@@ -104,6 +102,8 @@ struct cros_ec_device {
int dout_size;
struct device *parent;
bool wake_enabled;
+ int (*cmd_xfer)(struct cros_ec_device *ec,
+ struct cros_ec_command *msg);
struct mutex lock;
};
@@ -153,6 +153,18 @@ int cros_ec_check_result(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg);
/**
+ * cros_ec_cmd_xfer - Send a command to the ChromeOS EC
+ *
+ * Call this to send a command to the ChromeOS EC. This should be used
+ * instead of calling the EC's cmd_xfer() callback directly.
+ *
+ * @ec_dev: EC device
+ * @msg: Message to write
+ */
+int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg);
+
+/**
* cros_ec_remove - Remove a ChromeOS EC
*
* Call this to deregister a ChromeOS EC, then clean up any private data.
--
2.0.0.rc2
^ permalink raw reply related
* [PATCH 4/7] mfd: cros_ec: move locking into cros_ec_cmd_xfer
From: Javier Martinez Canillas @ 2014-07-28 12:19 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
Olof Johansson, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1406549967-21291-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
From: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Now that there's a central cros_ec_cmd_xfer(), move the locking
out of the SPI and LPC drivers.
Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
Reviewed-by: Simon Glass <sjg-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
drivers/mfd/cros_ec.c | 10 +++++++++-
drivers/mfd/cros_ec_spi.c | 11 -----------
2 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index a9faebd..c53804a 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -65,7 +65,13 @@ EXPORT_SYMBOL(cros_ec_check_result);
int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg)
{
- return ec_dev->cmd_xfer(ec_dev, msg);
+ int ret;
+
+ mutex_lock(&ec_dev->lock);
+ ret = ec_dev->cmd_xfer(ec_dev, msg);
+ mutex_unlock(&ec_dev->lock);
+
+ return ret;
}
EXPORT_SYMBOL(cros_ec_cmd_xfer);
@@ -98,6 +104,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
return -ENOMEM;
}
+ mutex_init(&ec_dev->lock);
+
err = mfd_add_devices(dev, 0, cros_devs,
ARRAY_SIZE(cros_devs),
NULL, ec_dev->irq, NULL);
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
index 6726f0a..2fcef1a 100644
--- a/drivers/mfd/cros_ec_spi.c
+++ b/drivers/mfd/cros_ec_spi.c
@@ -79,13 +79,11 @@
* if no record
* @end_of_msg_delay: used to set the delay_usecs on the spi_transfer that
* is sent when we want to turn off CS at the end of a transaction.
- * @lock: mutex to ensure only one user of cros_ec_cmd_xfer_spi at a time
*/
struct cros_ec_spi {
struct spi_device *spi;
s64 last_transfer_ns;
unsigned int end_of_msg_delay;
- struct mutex lock;
};
static void debug_packet(struct device *dev, const char *name, u8 *ptr,
@@ -233,13 +231,6 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
int ret = 0, final_ret;
struct timespec ts;
- /*
- * We have the shared ec_dev buffer plus we do lots of separate spi_sync
- * calls, so we need to make sure only one person is using this at a
- * time.
- */
- mutex_lock(&ec_spi->lock);
-
len = cros_ec_prepare_tx(ec_dev, ec_msg);
dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
@@ -331,7 +322,6 @@ exit:
if (ec_msg->command == EC_CMD_REBOOT_EC)
msleep(EC_REBOOT_DELAY_MS);
- mutex_unlock(&ec_spi->lock);
return ret;
}
@@ -363,7 +353,6 @@ static int cros_ec_spi_probe(struct spi_device *spi)
if (ec_spi == NULL)
return -ENOMEM;
ec_spi->spi = spi;
- mutex_init(&ec_spi->lock);
ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
if (!ec_dev)
return -ENOMEM;
--
2.0.0.rc2
^ permalink raw reply related
* [PATCH 5/7] mfd: cros_ec: wait for completion of commands that return IN_PROGRESS
From: Javier Martinez Canillas @ 2014-07-28 12:19 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
Olof Johansson, linux-i2c, linux-input, linux-samsung-soc
In-Reply-To: <1406549967-21291-1-git-send-email-javier.martinez@collabora.co.uk>
From: Andrew Bresticker <abrestic@chromium.org>
When an EC command returns EC_RES_IN_PROGRESS, we need to query
the state of the EC until it indicates that it is no longer busy.
Do this in cros_ec_cmd_xfer() under the EC's mutex so that other
commands (e.g. keyboard, I2C passtru) aren't issued to the EC while
it is working on the in-progress command.
Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
drivers/mfd/cros_ec.c | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index c53804a..634c434 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -23,6 +23,10 @@
#include <linux/mfd/core.h>
#include <linux/mfd/cros_ec.h>
#include <linux/mfd/cros_ec_commands.h>
+#include <linux/delay.h>
+
+#define EC_COMMAND_RETRIES 50
+#define EC_RETRY_DELAY_MS 10
int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg)
@@ -65,10 +69,39 @@ EXPORT_SYMBOL(cros_ec_check_result);
int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg)
{
- int ret;
+ int ret, i;
mutex_lock(&ec_dev->lock);
ret = ec_dev->cmd_xfer(ec_dev, msg);
+ if (ret == -EAGAIN && msg->result == EC_RES_IN_PROGRESS) {
+ /*
+ * Query the EC's status until it's no longer busy or
+ * we encounter an error.
+ */
+ for (i = 0; i < EC_COMMAND_RETRIES; i++) {
+ struct cros_ec_command status_msg;
+ struct ec_response_get_comms_status status;
+
+ msleep(EC_RETRY_DELAY_MS);
+
+ status_msg.version = 0;
+ status_msg.command = EC_CMD_GET_COMMS_STATUS;
+ status_msg.outdata = NULL;
+ status_msg.outsize = 0;
+ status_msg.indata = (uint8_t *)&status;
+ status_msg.insize = sizeof(status);
+
+ ret = ec_dev->cmd_xfer(ec_dev, &status_msg);
+ if (ret < 0)
+ break;
+
+ msg->result = status_msg.result;
+ if (status_msg.result != EC_RES_SUCCESS)
+ break;
+ if (!(status.flags & EC_COMMS_STATUS_PROCESSING))
+ break;
+ }
+ }
mutex_unlock(&ec_dev->lock);
return ret;
--
2.0.0.rc2
^ permalink raw reply related
* [PATCH 6/7] mfd: cros_ec: Instantiate sub-devices from device tree
From: Javier Martinez Canillas @ 2014-07-28 12:19 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
Olof Johansson, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1406549967-21291-1-git-send-email-javier.martinez-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
From: Todd Broch <tbroch-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
If the EC device tree node has sub-nodes, try to instantiate them as
MFD sub-devices. We can configure the EC features provided by the board.
Signed-off-by: Todd Broch <tbroch-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
---
drivers/mfd/cros_ec.c | 40 ++++++++++++++++++++++++++++++----------
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 634c434..96c926c 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -21,6 +21,7 @@
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/mfd/core.h>
+#include <linux/of_platform.h>
#include <linux/mfd/cros_ec.h>
#include <linux/mfd/cros_ec_commands.h>
#include <linux/delay.h>
@@ -109,22 +110,16 @@ int cros_ec_cmd_xfer(struct cros_ec_device *ec_dev,
EXPORT_SYMBOL(cros_ec_cmd_xfer);
static const struct mfd_cell cros_devs[] = {
- {
- .name = "cros-ec-keyb",
- .id = 1,
- .of_compatible = "google,cros-ec-keyb",
- },
- {
- .name = "cros-ec-i2c-tunnel",
- .id = 2,
- .of_compatible = "google,cros-ec-i2c-tunnel",
- },
};
int cros_ec_register(struct cros_ec_device *ec_dev)
{
struct device *dev = ec_dev->dev;
int err = 0;
+#ifdef CONFIG_OF
+ struct device_node *node;
+ int id = ARRAY_SIZE(cros_devs);
+#endif
if (ec_dev->din_size) {
ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
@@ -146,6 +141,31 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
dev_err(dev, "failed to add mfd devices\n");
return err;
}
+#ifdef CONFIG_OF
+ /*
+ * Add sub-devices declared in the device tree. NOTE they should NOT be
+ * declared in cros_devs
+ */
+ for_each_child_of_node(dev->of_node, node) {
+ char name[128];
+ struct mfd_cell cell = {
+ .id = 0,
+ .name = name,
+ };
+
+ if (of_modalias_node(node, name, sizeof(name)) < 0) {
+ dev_err(dev, "modalias failure on %s\n",
+ node->full_name);
+ continue;
+ }
+ dev_dbg(dev, "adding MFD sub-device %s\n", node->name);
+ cell.of_compatible = of_get_property(node, "compatible", NULL);
+ err = mfd_add_devices(dev, ++id, &cell, 1, NULL, ec_dev->irq,
+ NULL);
+ if (err)
+ dev_err(dev, "fail to add %s\n", node->full_name);
+ }
+#endif
dev_info(dev, "Chrome EC device registered\n");
--
2.0.0.rc2
^ permalink raw reply related
* [PATCH 7/7] Input: cros_ec_keyb: Optimize ghosting algorithm.
From: Javier Martinez Canillas @ 2014-07-28 12:19 UTC (permalink / raw)
To: Lee Jones
Cc: Wolfram Sang, Dmitry Torokhov, Doug Anderson, Simon Glass,
Bill Richardson, Andrew Bresticker, Derek Basehore, Todd Broch,
Olof Johansson, linux-i2c, linux-input, linux-samsung-soc
In-Reply-To: <1406549967-21291-1-git-send-email-javier.martinez@collabora.co.uk>
From: Todd Broch <tbroch@chromium.org>
Previous algorithm was a bit conservative and complicating with
respect to identifying key ghosting. This CL uses the bitops hamming
weight function (hweight8) to count the number of matching rows for
colM & colN. If that number is > 1 ghosting is present.
Additionally it removes NULL keys and our one virtual keypress
KEY_BATTERY from consideration as these inputs are never physical
keypresses.
Signed-off-by: Todd Broch <tbroch@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
---
drivers/input/keyboard/cros_ec_keyb.c | 92 +++++++++++++++++++----------------
1 file changed, 49 insertions(+), 43 deletions(-)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 93111d1..5d773d2 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -22,6 +22,7 @@
*/
#include <linux/module.h>
+#include <linux/bitops.h>
#include <linux/i2c.h>
#include <linux/input.h>
#include <linux/interrupt.h>
@@ -38,6 +39,7 @@
* @row_shift: log2 or number of rows, rounded up
* @keymap_data: Matrix keymap data used to convert to keyscan values
* @ghost_filter: true to enable the matrix key-ghosting filter
+ * @valid_keys: bitmap of existing keys for each matrix column
* @old_kb_state: bitmap of keys pressed last scan
* @dev: Device pointer
* @idev: Input device
@@ -49,6 +51,7 @@ struct cros_ec_keyb {
int row_shift;
const struct matrix_keymap_data *keymap_data;
bool ghost_filter;
+ uint8_t *valid_keys;
uint8_t *old_kb_state;
struct device *dev;
@@ -57,39 +60,15 @@ struct cros_ec_keyb {
};
-static bool cros_ec_keyb_row_has_ghosting(struct cros_ec_keyb *ckdev,
- uint8_t *buf, int row)
-{
- int pressed_in_row = 0;
- int row_has_teeth = 0;
- int col, mask;
-
- mask = 1 << row;
- for (col = 0; col < ckdev->cols; col++) {
- if (buf[col] & mask) {
- pressed_in_row++;
- row_has_teeth |= buf[col] & ~mask;
- if (pressed_in_row > 1 && row_has_teeth) {
- /* ghosting */
- dev_dbg(ckdev->dev,
- "ghost found at: r%d c%d, pressed %d, teeth 0x%x\n",
- row, col, pressed_in_row,
- row_has_teeth);
- return true;
- }
- }
- }
-
- return false;
-}
-
/*
* Returns true when there is at least one combination of pressed keys that
* results in ghosting.
*/
static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
{
- int row;
+ int col1, col2, buf1, buf2;
+ struct device *dev = ckdev->dev;
+ uint8_t *valid_keys = ckdev->valid_keys;
/*
* Ghosting happens if for any pressed key X there are other keys
@@ -103,27 +82,23 @@ static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
*
* In this case only X, Y, and Z are pressed, but g appears to be
* pressed too (see Wikipedia).
- *
- * We can detect ghosting in a single pass (*) over the keyboard state
- * by maintaining two arrays. pressed_in_row counts how many pressed
- * keys we have found in a row. row_has_teeth is true if any of the
- * pressed keys for this row has other pressed keys in its column. If
- * at any point of the scan we find that a row has multiple pressed
- * keys, and at least one of them is at the intersection with a column
- * with multiple pressed keys, we're sure there is ghosting.
- * Conversely, if there is ghosting, we will detect such situation for
- * at least one key during the pass.
- *
- * (*) This looks linear in the number of keys, but it's not. We can
- * cheat because the number of rows is small.
*/
- for (row = 0; row < ckdev->rows; row++)
- if (cros_ec_keyb_row_has_ghosting(ckdev, buf, row))
- return true;
+ for (col1 = 0; col1 < ckdev->cols; col1++) {
+ buf1 = buf[col1] & valid_keys[col1];
+ for (col2 = col1 + 1; col2 < ckdev->cols; col2++) {
+ buf2 = buf[col2] & valid_keys[col2];
+ if (hweight8(buf1 & buf2) > 1) {
+ dev_dbg(dev, "ghost found at: B[%02d]:0x%02x & B[%02d]:0x%02x",
+ col1, buf1, col2, buf2);
+ return true;
+ }
+ }
+ }
return false;
}
+
/*
* Compares the new keyboard state to the old one and produces key
* press/release events accordingly. The keyboard state is 13 bytes (one byte
@@ -222,6 +197,30 @@ static void cros_ec_keyb_close(struct input_dev *dev)
free_irq(ec->irq, ckdev);
}
+/*
+ * Walks keycodes flipping bit in buffer COLUMNS deep where bit is ROW. Used by
+ * ghosting logic to ignore NULL or virtual keys.
+ */
+static void cros_ec_keyb_compute_valid_keys(struct cros_ec_keyb *ckdev)
+{
+ int row, col;
+ int row_shift = ckdev->row_shift;
+ unsigned short *keymap = ckdev->idev->keycode;
+ unsigned short code;
+
+ BUG_ON(ckdev->idev->keycodesize != sizeof(*keymap));
+
+ for (col = 0; col < ckdev->cols; col++) {
+ for (row = 0; row < ckdev->rows; row++) {
+ code = keymap[MATRIX_SCAN_CODE(row, col, row_shift)];
+ if (code && (code != KEY_BATTERY))
+ ckdev->valid_keys[col] |= 1 << row;
+ }
+ dev_dbg(ckdev->dev, "valid_keys[%02d] = 0x%02x\n",
+ col, ckdev->valid_keys[col]);
+ }
+}
+
static int cros_ec_keyb_probe(struct platform_device *pdev)
{
struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
@@ -242,6 +241,11 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
&ckdev->cols);
if (err)
return err;
+
+ ckdev->valid_keys = devm_kzalloc(&pdev->dev, ckdev->cols, GFP_KERNEL);
+ if (!ckdev->valid_keys)
+ return -ENOMEM;
+
ckdev->old_kb_state = devm_kzalloc(&pdev->dev, ckdev->cols, GFP_KERNEL);
if (!ckdev->old_kb_state)
return -ENOMEM;
@@ -285,6 +289,8 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
input_set_capability(idev, EV_MSC, MSC_SCAN);
input_set_drvdata(idev, ckdev);
ckdev->idev = idev;
+ cros_ec_keyb_compute_valid_keys(ckdev);
+
err = input_register_device(ckdev->idev);
if (err) {
dev_err(dev, "cannot register input device\n");
--
2.0.0.rc2
^ permalink raw reply related
* Re: [PATCHES v2] Add support for more Huion tablets
From: Benjamin Tissoires @ 2014-07-28 15:33 UTC (permalink / raw)
To: Nikolai Kondrashov; +Cc: Jiri Kosina, linux-input, DIGImend-devel
In-Reply-To: <1406133117-29243-1-git-send-email-spbnick@gmail.com>
On Wed, Jul 23, 2014 at 12:31 PM, Nikolai Kondrashov <spbnick@gmail.com> wrote:
> Hi everyone,
>
> This is the second version of the patch set adding support for more Huion
> tablets [1].
>
> This version has the "hid: huion: Invert in-range on specific product" patch
> removed as requested by Benjamin Tissoires.
>
> Tested with Huion H610N.
>
> Nick
>
> [PATCH 1/4] hid: huion: Use "tablet" instead of specific model
> [PATCH 2/4] hid: huion: Don't ignore other interfaces
> [PATCH 3/4] hid: huion: Switch to generating report descriptor
> [PATCH 4/4] hid: huion: Handle tablets with UC-Logic vendor ID
>
> drivers/hid/hid-core.c | 2 +-
> drivers/hid/hid-huion.c | 265 ++++++++++++++++++++++++++++++++----------------
> drivers/hid/hid-ids.h | 2 +-
> 3 files changed, 177 insertions(+), 92 deletions(-)
>
> [1] http://thread.gmane.org/gmane.linux.kernel.input/37187
Just so that the info does not get lost, the v2 of the patch series is still:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cheers,
Benjamin
^ permalink raw reply
* Re: Linux 3.16-rc6
From: Waiman Long @ 2014-07-28 16:37 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Linus Torvalds, Borislav Petkov, Ingo Molnar,
Linux Kernel Mailing List, USB list, linux-input@vger.kernel.org
In-Reply-To: <20140725161002.GE6758@twins.programming.kicks-ass.net>
On 07/25/2014 12:10 PM, Peter Zijlstra wrote:
> On Thu, Jul 24, 2014 at 04:38:28PM -0400, Waiman Long wrote:
>> Yes, I think I may have a solution for that.
>>
>> Borislav, can you apply the following patch on top of the lockdep patch to
>> see if it can fix the problem?
>>
>> diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
>> index d24e433..507a8ce 100644
>> --- a/kernel/locking/lockdep.c
>> +++ b/kernel/locking/lockdep.c
>> @@ -3595,6 +3595,12 @@ void lock_acquire(struct lockdep_map *lock, unsigned int
>> raw_local_irq_save(flags);
>> check_flags(flags);
>>
>> + /*
>> + * An interrupt recursive read in interrupt context can be considered
>> + * to be the same as a recursive read from checking perspective.
>> + */
>> + if ((read == 3)&& in_interrupt())
>> + read = 2;
>> current->lockdep_recursion = 1;
>> trace_lock_acquire(lock, subclass, trylock, read, check, nest_lock, ip);
>> __lock_acquire(lock, subclass, trylock, read, check,
> Just had another look at the initial patch and it cannot be right, even
> with the above.
>
> The problem is you cannot use in_interrupt() in check_deadlock().
> Check_deadlock() must be context invariant, it should only test the
> chain state and not rely on where or when its called.
>
>
I am planning to take out the check in check_deadlock and only have the
test in lock_acquire which change a 3 to 2 when in interrupt context.
Now my question is whether to do it as a new patch on top of the
existing one in tip or a total replacement. I also intend to use
symbolic names for the read states for better readability as suggested
by John.
-Longman
^ permalink raw reply
* Re: Linux 3.16-rc6
From: Peter Zijlstra @ 2014-07-28 16:42 UTC (permalink / raw)
To: Waiman Long
Cc: Linus Torvalds, Borislav Petkov, Ingo Molnar,
Linux Kernel Mailing List, USB list, linux-input@vger.kernel.org
In-Reply-To: <53D67C3A.1060401@hp.com>
[-- Attachment #1: Type: text/plain, Size: 765 bytes --]
On Mon, Jul 28, 2014 at 12:37:14PM -0400, Waiman Long wrote:
> I am planning to take out the check in check_deadlock and only have the test
> in lock_acquire which change a 3 to 2 when in interrupt context. Now my
> question is whether to do it as a new patch on top of the existing one in
> tip or a total replacement. I also intend to use symbolic names for the read
> states for better readability as suggested by John.
Send new patches, the patches magically went away from tip.
I don't care too much about the symbolic thing, partly because the
actual value is not irrelevant seeing how we're peddling with bitfields.
Also, its an unrelated cleanup at best.
When you do re-submit extend the locking self test scenarios to cover
the new semantics as well.
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] input: ads7846: Switch to managed version of kzalloc and cleanups
From: Dmitry Torokhov @ 2014-07-28 16:49 UTC (permalink / raw)
To: pramod.gurav.etc
Cc: linux-input, linux-kernel, Pramod Gurav, Guenter Roeck,
Paul Gortmaker, Mark Brown, Fabio Estevam
In-Reply-To: <1406537216-25936-1-git-send-email-pramod.gurav@smartplayin.com>
Hi Pramod,
On Mon, Jul 28, 2014 at 02:16:55PM +0530, pramod.gurav.etc@gmail.com wrote:
> From: Pramod Gurav <pramod.gurav@smartplayin.com>
>
> This switches memory allocations from kzalloc to devm_kzalloc.
> This also changes the way return checks were done on failure cases
> of three allocations. The checks were clubbed together and hence
> must be done seperately to avoid calling kfree on unallocated memory.
>
> Moreover in case of input_allocate_device failure, we were calling
> input_free_device which is not needed.
But not hurting either - like kfree() and many other "cleanup" functions
in kernel they are happily accept NULL input (so that cleanup code is
less branchy).
>
> input device must be released(input_free_device) when ads7846_probe_dt
> fails hence adds a fix there as well.
That is the real bug, could you send me a patch that fixes just that (by
jumping to err_free_mem)?
The rest I'd like to leave as is unless you can convert _all_ resources
to be managed ones: mixing up 2 styles (manual and automatic release) is
bound to have issues (like we had with ads7846_probe_dt which assumed
that since it was releasing its resources automatically the rest would
be released automatically as well.
Thanks.
--
Dmitry
^ permalink raw reply
* [v2] input: drv260x: Add TI drv260x haptics driver
From: Dan Murphy @ 2014-07-28 16:53 UTC (permalink / raw)
To: linux-input-u79uwXL29TY76Z2rM5mHXA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w, Dan Murphy
Add the TI drv260x haptics/vibrator driver.
This device uses the input force feedback
to produce a wave form to driver an
ERM or LRA actuator device.
The initial driver supports the devices
real time playback mode. But the device
has additional wave patterns in ROM.
This functionality will be added in
future patchsets.
Product data sheet is located here:
http://www.ti.com/product/drv2605
Signed-off-by: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
---
v2 - Fixed binding doc and patch headline - https://patchwork.kernel.org/patch/4619641/
.../devicetree/bindings/input/ti,drv260x.txt | 44 ++
drivers/input/misc/Kconfig | 9 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/drv260x.c | 537 ++++++++++++++++++++
include/dt-bindings/input/ti-drv260x.h | 30 ++
include/linux/input/drv260x.h | 181 +++++++
6 files changed, 802 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/ti,drv260x.txt
create mode 100644 drivers/input/misc/drv260x.c
create mode 100644 include/dt-bindings/input/ti-drv260x.h
create mode 100644 include/linux/input/drv260x.h
diff --git a/Documentation/devicetree/bindings/input/ti,drv260x.txt b/Documentation/devicetree/bindings/input/ti,drv260x.txt
new file mode 100644
index 0000000..930429b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/ti,drv260x.txt
@@ -0,0 +1,44 @@
+Texas Instruments - drv260x Haptics driver family
+
+The drv260x family serial control bus communicates through I2C protocols
+
+Required properties:
+ - compatible - One of:
+ "ti,drv2604" - DRV2604
+ "ti,drv2605" - DRV2605
+ "ti,drv2605l" - DRV2605L
+ - reg - I2C slave address
+ - mode - Power up mode of the chip (defined in include/dt-bindings/input/ti-drv260x.h)
+ DRV260X_RTP_MODE - Real time playback mode
+ DRV260X_LRA_MODE - Linear Resonance Actuator mode (Piezoelectric)
+ DRV260X_ERM_MODE - Eccentric Rotating Mass mode (Rotary vibrator)
+ - library_sel - Library to use at power up (defined in include/dt-bindings/input/ti-drv260x.h)
+ DRV260X_LIB_A - Pre-programmed Library
+ DRV260X_LIB_B - Pre-programmed Library
+ DRV260X_LIB_C - Pre-programmed Library
+ DRV260X_LIB_D - Pre-programmed Library
+ DRV260X_LIB_E - Pre-programmed Library
+ DRV260X_LIB_F - Pre-programmed Library
+
+Optional properties:
+ - enable-gpio - gpio pin to enable/disable the device.
+ - vib_rated_voltage - The rated voltage of the actuator. If this is not
+ set then the value will be defaulted to 0x90 in the
+ driver.
+ - vib_overdrive_voltage - The overdrive voltage of the actuator.
+ If this is not set then the value
+ will be defaulted to 0x90 in the driver.
+Example:
+
+drv2605l: drv2605l@5a {
+ compatible = "ti,drv2605l";
+ reg = <0x5a>;
+ enable-gpio = <&gpio1 28 GPIO_ACTIVE_HIGH>;
+ mode = <DRV260X_RTP_MODE>;
+ library_sel = <DRV260X_LIB_SEL_DEFAULT>;
+ vib_rated_voltage = <3200>;
+ vib_overdriver_voltage = <3200>;
+};
+
+For more product information please see the link below:
+http://www.ti.com/product/drv2605
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 2ff4425..99f6762 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -676,4 +676,13 @@ config INPUT_SOC_BUTTON_ARRAY
To compile this driver as a module, choose M here: the
module will be called soc_button_array.
+config INPUT_DRV260X_HAPTICS
+ tristate "TI DRV260X haptics support"
+ depends on INPUT && I2C
+ help
+ Say Y to enable support for the TI DRV260X haptics driver.
+
+ To compile this driver as a module, choose M here: the
+ module will be called drv260x-haptics.
+
endif
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 4955ad3..d8ef3c7 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -64,3 +64,4 @@ obj-$(CONFIG_INPUT_WM831X_ON) += wm831x-on.o
obj-$(CONFIG_INPUT_XEN_KBDDEV_FRONTEND) += xen-kbdfront.o
obj-$(CONFIG_INPUT_YEALINK) += yealink.o
obj-$(CONFIG_INPUT_IDEAPAD_SLIDEBAR) += ideapad_slidebar.o
+obj-$(CONFIG_INPUT_DRV260X_HAPTICS) += drv260x.o
diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
new file mode 100644
index 0000000..aadfd57
--- /dev/null
+++ b/drivers/input/misc/drv260x.c
@@ -0,0 +1,537 @@
+/*
+ * drv260x.c - DRV260X haptics driver family
+ *
+ * Author: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
+ *
+ * Copyright: (C) 2014 Texas Instruments, Inc.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/regulator/consumer.h>
+
+#include <linux/input/drv260x.h>
+#include <dt-bindings/input/ti-drv260x.h>
+
+#define DRV260X_MAX_EFFECTS 255
+
+/**
+ * struct drv260x_data -
+ * @input_dev - Pointer to the input device
+ * @client - Pointer to the I2C client
+ * @timer - High res timer used to turn on/off vibration
+ * @regmap - Register map of the device
+ * @lock - Lock for device access
+ * @work - Work item used to off load the enable/disable of the vibration
+ * @enable_gpio - Pointer to the gpio used for enable/disabling
+ * @regulator - Pointer to the regulator for the IC
+ * @upload_effects - Vibration effect data array
+ * @runtime_left - Indicates how much runtime is remaining in the timer
+ * @effect_id - The ID of the vibration effect requested to be played
+ * @mode - The operating mode of the IC (RTP, ERM or LRA)
+ * @library - The vibration library to be used
+ * @rated_voltage - The rated_voltage of the actuator
+ * @overdriver_voltage - The over drive voltage of the actuator
+**/
+struct drv260x_data {
+ struct input_dev *input_dev;
+ struct i2c_client *client;
+ struct hrtimer timer;
+ struct regmap *regmap;
+ struct mutex lock;
+ struct work_struct work;
+ struct gpio_desc *enable_gpio;
+ struct regulator *regulator;
+ struct ff_effect upload_effects[DRV260X_MAX_EFFECTS];
+ u32 runtime_left;
+ u32 effect_id;
+ int mode;
+ int library;
+ int rated_voltage;
+ int overdrive_voltage;
+};
+
+static struct reg_default drv260x_reg_defs[] = {
+ { DRV260X_STATUS, 0xe0 },
+ { DRV260X_MODE, 0x40 },
+ { DRV260X_RT_PB_IN, 0x00},
+ { DRV260X_LIB_SEL, 0x00},
+ { DRV260X_WV_SEQ_1, 0x01},
+ { DRV260X_WV_SEQ_2, 0x00},
+ { DRV260X_WV_SEQ_3, 0x00},
+ { DRV260X_WV_SEQ_4, 0x00},
+ { DRV260X_WV_SEQ_5, 0x00},
+ { DRV260X_WV_SEQ_6, 0x00},
+ { DRV260X_WV_SEQ_7, 0x00},
+ { DRV260X_WV_SEQ_8, 0x00},
+ { DRV260X_GO, 0x00},
+ { DRV260X_OVERDRIVE_OFF, 0x00},
+ { DRV260X_SUSTAIN_P_OFF, 0x00},
+ { DRV260X_SUSTAIN_N_OFF, 0x00},
+ { DRV260X_BRAKE_OFF , 0x00},
+ { DRV260X_A_TO_V_CTRL , 0x05},
+ { DRV260X_A_TO_V_MIN_INPUT, 0x19},
+ { DRV260X_A_TO_V_MAX_INPUT, 0xff},
+ { DRV260X_A_TO_V_MIN_OUT, 0x19},
+ { DRV260X_A_TO_V_MAX_OUT, 0xff},
+ { DRV260X_RATED_VOLT, 0x3e},
+ { DRV260X_OD_CLAMP_VOLT, 0x8c},
+ { DRV260X_CAL_COMP, 0x0c},
+ { DRV260X_CAL_BACK_EMF, 0x6c},
+ { DRV260X_FEEDBACK_CTRL, 0x36},
+ { DRV260X_CTRL1, 0x93},
+ { DRV260X_CTRL2, 0xfa},
+ { DRV260X_CTRL3, 0xa0},
+ { DRV260X_CTRL4, 0x20},
+ { DRV260X_CTRL5, 0x80},
+ { DRV260X_LRA_LOOP_PERIOD, 0x33},
+ { DRV260X_VBAT_MON, 0x00},
+ { DRV260X_LRA_RES_PERIOD, 0x00},
+};
+
+/**
+ * Rated and Overdriver Voltages:
+ * Calculated using the formula r = v * 255 / 5.6
+ * where r is what will be written to the register
+ * and v is the rated or overdriver voltage of the actuator
+ **/
+#define DRV260X_DEF_RATED_VOLT 0x90
+#define DRV260X_DEF_OD_CLAMP_VOLT 0x90
+
+static int drv260x_calculate_voltage(int voltage)
+{
+ return (voltage * 255 / 5600);
+}
+
+static void drv260x_worker(struct work_struct *work)
+{
+ struct drv260x_data *haptics = container_of(work, struct drv260x_data, work);
+ int effect_id = haptics->effect_id;
+ int magnitude;
+
+ mutex_lock(&haptics->lock);
+
+ if (haptics->upload_effects[effect_id].u.rumble.strong_magnitude > 0)
+ magnitude = haptics->upload_effects[effect_id].u.rumble.strong_magnitude;
+ else if (haptics->upload_effects[effect_id].u.rumble.weak_magnitude > 0)
+ magnitude = haptics->upload_effects[effect_id].u.rumble.weak_magnitude;
+ else
+ goto out;
+
+ if (haptics->runtime_left > 0)
+ regmap_write(haptics->regmap, DRV260X_RT_PB_IN, magnitude);
+ else
+ regmap_write(haptics->regmap, DRV260X_RT_PB_IN, 0x0);
+
+out:
+ mutex_unlock(&haptics->lock);
+}
+
+static enum hrtimer_restart drv260x_timer(struct hrtimer *timer)
+{
+ struct drv260x_data *data = container_of(timer, struct drv260x_data, timer);
+
+ data->runtime_left = 0;
+ schedule_work(&data->work);
+
+ return HRTIMER_NORESTART;
+}
+
+static int drv260x_playback(struct input_dev *input, int effect_id, int value)
+{
+ struct drv260x_data *haptics = input_get_drvdata(input);
+ unsigned long time_ms;
+
+ gpiod_set_value(haptics->enable_gpio, 1);
+ /* Data sheet says to wait 250us before trying to communicate */
+ udelay(250);
+
+ regmap_write(haptics->regmap, DRV260X_MODE, DRV260X_RT_PLAYBACK);
+
+ haptics->mode = DRV260X_RTP_MODE;
+ haptics->runtime_left = haptics->upload_effects[effect_id].replay.length;
+ haptics->effect_id = effect_id;
+
+ time_ms = haptics->runtime_left * NSEC_PER_MSEC;
+
+ schedule_work(&haptics->work);
+ hrtimer_start(&haptics->timer, ktime_set(0, time_ms), HRTIMER_MODE_REL);
+
+ return 0;
+}
+
+static int drv260x_upload_effect(struct input_dev *input,
+ struct ff_effect *effect,
+ struct ff_effect *old)
+{
+ struct drv260x_data *haptics = input_get_drvdata(input);
+
+ haptics->upload_effects[effect->id] = *effect;
+
+ return 0;
+}
+
+static void drv260x_close(struct input_dev *input)
+{
+ struct drv260x_data *haptics = input_get_drvdata(input);
+
+ cancel_work_sync(&haptics->work);
+ regmap_write(haptics->regmap, DRV260X_MODE, DRV260X_STANDBY);
+ gpiod_set_value(haptics->enable_gpio, 0);
+}
+
+static const struct reg_default drv260x_lra_cal_regs[] = {
+ { DRV260X_MODE, DRV260X_AUTO_CAL },
+ { DRV260X_CTRL3, DRV260X_NG_THRESH_2},
+ { DRV260X_FEEDBACK_CTRL, DRV260X_FB_REG_LRA_MODE | DRV260X_BRAKE_FACTOR_4X | DRV260X_LOOP_GAIN_HIGH },
+};
+
+static const struct reg_default drv260x_lra_init_regs[] = {
+ { DRV260X_MODE, DRV260X_RT_PLAYBACK},
+ { DRV260X_LIB_SEL, DRV260X_LIB_F },
+ { DRV260X_A_TO_V_CTRL, DRV260X_AUDIO_HAPTICS_PEAK_20MS | DRV260X_AUDIO_HAPTICS_FILTER_125HZ},
+ { DRV260X_A_TO_V_MIN_INPUT, DRV260X_AUDIO_HAPTICS_MIN_IN_VOLT },
+ { DRV260X_A_TO_V_MAX_INPUT, DRV260X_AUDIO_HAPTICS_MAX_IN_VOLT },
+ { DRV260X_A_TO_V_MIN_OUT, DRV260X_AUDIO_HAPTICS_MIN_OUT_VOLT },
+ { DRV260X_A_TO_V_MAX_OUT, DRV260X_AUDIO_HAPTICS_MAX_OUT_VOLT },
+ { DRV260X_FEEDBACK_CTRL, DRV260X_FB_REG_LRA_MODE | DRV260X_BRAKE_FACTOR_2X | DRV260X_LOOP_GAIN_MED | DRV260X_BEMF_GAIN_3 },
+ { DRV260X_CTRL1, DRV260X_STARTUP_BOOST },
+ { DRV260X_CTRL2, DRV260X_SAMP_TIME_250 },
+ { DRV260X_CTRL3, DRV260X_NG_THRESH_2 | DRV260X_ANANLOG_IN },
+ { DRV260X_CTRL4, DRV260X_AUTOCAL_TIME_500MS },
+};
+
+static const struct reg_default drv260x_erm_cal_regs[] = {
+ { DRV260X_MODE, DRV260X_AUTO_CAL },
+ { DRV260X_A_TO_V_MIN_INPUT, DRV260X_AUDIO_HAPTICS_MIN_IN_VOLT },
+ { DRV260X_A_TO_V_MAX_INPUT, DRV260X_AUDIO_HAPTICS_MAX_IN_VOLT },
+ { DRV260X_A_TO_V_MIN_OUT, DRV260X_AUDIO_HAPTICS_MIN_OUT_VOLT },
+ { DRV260X_A_TO_V_MAX_OUT, DRV260X_AUDIO_HAPTICS_MAX_OUT_VOLT },
+ { DRV260X_FEEDBACK_CTRL, DRV260X_BRAKE_FACTOR_3X | DRV260X_LOOP_GAIN_MED | DRV260X_BEMF_GAIN_2 },
+ { DRV260X_CTRL1, DRV260X_STARTUP_BOOST },
+ { DRV260X_CTRL2, DRV260X_SAMP_TIME_250 | DRV260X_BLANK_TIME_75 | DRV260X_SAMP_TIME_250 | DRV260X_IDISS_TIME_75 },
+ { DRV260X_CTRL3, DRV260X_NG_THRESH_2 | DRV260X_ERM_OPEN_LOOP },
+ { DRV260X_CTRL4, DRV260X_AUTOCAL_TIME_500MS },
+};
+
+static int drv260x_init(struct drv260x_data *haptics)
+{
+ int ret;
+ unsigned int cal_buf;
+
+ ret = regmap_write(haptics->regmap,
+ DRV260X_RATED_VOLT, haptics->rated_voltage);
+ if (ret != 0)
+ goto write_failure;
+
+ ret = regmap_write(haptics->regmap,
+ DRV260X_OD_CLAMP_VOLT, haptics->overdrive_voltage);
+ if (ret != 0)
+ goto write_failure;
+
+ if (haptics->mode == DRV260X_LRA_MODE) {
+ ret = regmap_register_patch(haptics->regmap,
+ drv260x_lra_cal_regs,
+ ARRAY_SIZE(drv260x_lra_cal_regs));
+ if (ret != 0)
+ goto write_failure;
+
+ } else if (haptics->mode == DRV260X_ERM_MODE) {
+ ret = regmap_register_patch(haptics->regmap,
+ drv260x_erm_cal_regs,
+ ARRAY_SIZE(drv260x_erm_cal_regs));
+ if (ret != 0)
+ goto write_failure;
+
+ ret = regmap_update_bits(haptics->regmap, DRV260X_LIB_SEL,
+ DRV260X_LIB_SEL_MASK,
+ haptics->library);
+ if (ret != 0)
+ goto write_failure;
+
+ } else {
+ ret = regmap_register_patch(haptics->regmap,
+ drv260x_lra_init_regs,
+ ARRAY_SIZE(drv260x_lra_init_regs));
+ if (ret != 0)
+ goto write_failure;
+
+ goto skip_go_bit;
+ }
+
+ if (ret != 0) {
+ dev_err(&haptics->client->dev,
+ "Failed to write init registers: %d\n",
+ ret);
+ goto write_failure;
+ }
+
+ ret = regmap_write(haptics->regmap, DRV260X_GO, DRV260X_GO_BIT);
+ if (ret != 0)
+ goto write_failure;
+
+ do {
+ ret = regmap_read(haptics->regmap, DRV260X_GO, &cal_buf);
+ if (ret != 0)
+ goto write_failure;
+ } while (cal_buf == DRV260X_GO_BIT || ret != 0);
+
+ return ret;
+
+write_failure:
+ dev_err(&haptics->client->dev,
+ "Failed to write init registers: %d\n",
+ ret);
+skip_go_bit:
+ return ret;
+}
+
+static const struct regmap_config drv260x_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+
+ .max_register = DRV260X_MAX_REG,
+ .reg_defaults = drv260x_reg_defs,
+ .num_reg_defaults = ARRAY_SIZE(drv260x_reg_defs),
+ .cache_type = REGCACHE_NONE,
+};
+
+static int drv260x_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ struct drv260x_data *haptics;
+ struct device_node *np = client->dev.of_node;
+ struct drv260x_platform_data *pdata = client->dev.platform_data;
+ struct ff_device *ff;
+ int ret;
+ int voltage;
+
+ haptics = devm_kzalloc(&client->dev, sizeof(*haptics), GFP_KERNEL);
+ if (!haptics)
+ return -ENOMEM;
+
+ haptics->rated_voltage = DRV260X_DEF_OD_CLAMP_VOLT;
+ haptics->rated_voltage = DRV260X_DEF_RATED_VOLT;
+
+ if (np) {
+ ret = of_property_read_u32(np, "mode", &haptics->mode);
+ if (ret < 0) {
+ dev_err(&client->dev,
+ "%s: No entry for mode\n", __func__);
+
+ return ret;
+ }
+ ret = of_property_read_u32(np, "library_sel",
+ &haptics->library);
+ if (ret < 0) {
+ dev_err(&client->dev,
+ "%s: No entry for library selection\n", __func__);
+
+ return ret;
+ }
+ ret = of_property_read_u32(np, "vib_rated_voltage",
+ &voltage);
+ if (!ret)
+ haptics->rated_voltage = drv260x_calculate_voltage(voltage);
+
+
+ ret = of_property_read_u32(np, "vib_overdrive_voltage",
+ &voltage);
+ if (!ret)
+ haptics->overdrive_voltage = drv260x_calculate_voltage(voltage);
+
+ } else if (pdata) {
+ haptics->mode = pdata->mode;
+ haptics->library = pdata->library_selection;
+ if (pdata->vib_overdrive_voltage)
+ haptics->overdrive_voltage = drv260x_calculate_voltage(pdata->vib_overdrive_voltage);
+ if (pdata->vib_rated_voltage)
+ haptics->rated_voltage = drv260x_calculate_voltage(pdata->vib_rated_voltage);
+ } else {
+ dev_err(&client->dev, "Platform data not set\n");
+ return -ENODEV;
+ }
+
+ haptics->regulator = regulator_get(&client->dev, "vbat");
+ if (IS_ERR(haptics->regulator)) {
+ ret = PTR_ERR(haptics->regulator);
+ dev_err(&client->dev,
+ "unable to get regulator, error: %d\n", ret);
+ goto err_regulator;
+ }
+
+ haptics->enable_gpio = devm_gpiod_get(&client->dev, "enable");
+ if (IS_ERR(haptics->enable_gpio)) {
+ ret = PTR_ERR(haptics->enable_gpio);
+ if (ret != -ENOENT && ret != -ENOSYS)
+ goto err_gpio;
+
+ haptics->enable_gpio = NULL;
+ } else {
+ gpiod_direction_output(haptics->enable_gpio, 1);
+ }
+
+ haptics->input_dev = input_allocate_device();
+ if (haptics->input_dev == NULL) {
+ dev_err(&client->dev, "Failed to allocate input device\n");
+ ret = -ENOMEM;
+ goto err_input_alloc;
+ }
+
+ haptics->input_dev->name = "drv260x:haptics";
+ haptics->input_dev->dev.parent = client->dev.parent;
+ haptics->input_dev->close = drv260x_close;
+ input_set_drvdata(haptics->input_dev, haptics);
+ input_set_capability(haptics->input_dev, EV_FF, FF_RUMBLE);
+
+ ret = input_ff_create(haptics->input_dev, DRV260X_MAX_EFFECTS);
+ if (ret < 0) {
+ dev_err(&client->dev, "input_ff_create() failed: %d\n",
+ ret);
+ goto err_ff_create;
+ }
+
+ ff = haptics->input_dev->ff;
+ ff->playback = drv260x_playback;
+ ff->upload = drv260x_upload_effect;
+
+ mutex_init(&haptics->lock);
+ INIT_WORK(&haptics->work, drv260x_worker);
+ hrtimer_init(&haptics->timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ haptics->timer.function = drv260x_timer;
+
+ haptics->client = client;
+ i2c_set_clientdata(client, haptics);
+
+ haptics->regmap = devm_regmap_init_i2c(client, &drv260x_regmap_config);
+ if (IS_ERR(haptics->regmap)) {
+ ret = PTR_ERR(haptics->regmap);
+ dev_err(&client->dev, "Failed to allocate register map: %d\n",
+ ret);
+ goto err_regmap;
+ }
+
+ drv260x_init(haptics);
+
+ ret = input_register_device(haptics->input_dev);
+ if (ret < 0) {
+ dev_err(&client->dev, "couldn't register input device: %d\n",
+ ret);
+ goto err_iff;
+ }
+ return 0;
+
+err_iff:
+err_regmap:
+ if (haptics->input_dev)
+ input_ff_destroy(haptics->input_dev);
+err_ff_create:
+ input_free_device(haptics->input_dev);
+err_input_alloc:
+err_gpio:
+ regulator_put(haptics->regulator);
+err_regulator:
+ return ret;
+}
+
+static int drv260x_remove(struct i2c_client *client)
+{
+ struct drv260x_data *haptics = i2c_get_clientdata(client);
+
+ cancel_work_sync(&haptics->work);
+
+ regmap_write(haptics->regmap, DRV260X_MODE, DRV260X_STANDBY);
+ gpiod_set_value(haptics->enable_gpio, 0);
+
+ input_unregister_device(haptics->input_dev);
+ regulator_put(haptics->regulator);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int drv260x_suspend(struct device *dev)
+{
+ struct drv260x_data *haptics = dev_get_drvdata(dev);
+
+ regmap_update_bits(haptics->regmap,
+ DRV260X_MODE,
+ DRV260X_STANDBY_MASK,
+ DRV260X_STANDBY);
+ gpiod_set_value(haptics->enable_gpio, 0);
+
+ regulator_disable(haptics->regulator);
+
+ return 0;
+}
+
+static int drv260x_resume(struct device *dev)
+{
+ struct drv260x_data *haptics = dev_get_drvdata(dev);
+ int ret;
+
+ ret = regulator_enable(haptics->regulator);
+ if (ret) {
+ dev_err(dev, "Failed to enable regulator\n");
+ return ret;
+ }
+ regmap_update_bits(haptics->regmap,
+ DRV260X_MODE,
+ DRV260X_STANDBY_MASK, 0);
+
+ gpiod_set_value(haptics->enable_gpio, 1);
+
+ return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(drv260x_pm_ops, drv260x_suspend, drv260x_resume);
+
+static const struct i2c_device_id drv260x_id[] = {
+ { "drv2605l", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, drv260x_id);
+
+#if IS_ENABLED(CONFIG_OF)
+static const struct of_device_id drv260x_of_match[] = {
+ { .compatible = "ti,drv2604", },
+ { .compatible = "ti,drv2605", },
+ { .compatible = "ti,drv2605l", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, drv260x_of_match);
+#endif
+
+static struct i2c_driver drv260x_driver = {
+ .probe = drv260x_probe,
+ .remove = drv260x_remove,
+ .driver = {
+ .name = "drv260x-haptics",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(drv260x_of_match),
+ .pm = &drv260x_pm_ops,
+ },
+ .id_table = drv260x_id,
+};
+module_i2c_driver(drv260x_driver);
+
+MODULE_ALIAS("platform:drv260x-haptics");
+MODULE_DESCRIPTION("TI DRV260x haptics driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>");
diff --git a/include/dt-bindings/input/ti-drv260x.h b/include/dt-bindings/input/ti-drv260x.h
new file mode 100644
index 0000000..28f1dfa
--- /dev/null
+++ b/include/dt-bindings/input/ti-drv260x.h
@@ -0,0 +1,30 @@
+/*
+ * ti-drv260x.h - DRV260X haptics driver family
+ *
+ * Author: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
+ *
+ * Copyright: (C) 2014 Texas Instruments, Inc.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+/* Calibration Types */
+#define DRV260X_RTP_MODE 0x00
+#define DRV260X_LRA_MODE 0x01
+#define DRV260X_ERM_MODE 0x02
+
+/* Library Selection */
+#define DRV260X_LIB_SEL_DEFAULT 0x00
+#define DRV260X_LIB_A 0x01
+#define DRV260X_LIB_B 0x02
+#define DRV260X_LIB_C 0x03
+#define DRV260X_LIB_D 0x04
+#define DRV260X_LIB_E 0x05
+#define DRV260X_LIB_F 0x06
diff --git a/include/linux/input/drv260x.h b/include/linux/input/drv260x.h
new file mode 100644
index 0000000..709395e
--- /dev/null
+++ b/include/linux/input/drv260x.h
@@ -0,0 +1,181 @@
+/*
+ * drv260x.h - DRV260X haptics driver family
+ *
+ * Author: Dan Murphy <dmurphy-l0cyMroinI0@public.gmane.org>
+ *
+ * Copyright: (C) 2014 Texas Instruments, Inc.
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+
+#ifndef _LINUX_DRV260X_I2C_H
+#define _LINUX_DRV260X_I2C_H
+
+#define DRV260X_STATUS 0x0
+#define DRV260X_MODE 0x1
+#define DRV260X_RT_PB_IN 0x2
+#define DRV260X_LIB_SEL 0x3
+#define DRV260X_WV_SEQ_1 0x4
+#define DRV260X_WV_SEQ_2 0x5
+#define DRV260X_WV_SEQ_3 0x6
+#define DRV260X_WV_SEQ_4 0x7
+#define DRV260X_WV_SEQ_5 0x8
+#define DRV260X_WV_SEQ_6 0x9
+#define DRV260X_WV_SEQ_7 0xa
+#define DRV260X_WV_SEQ_8 0xb
+#define DRV260X_GO 0xc
+#define DRV260X_OVERDRIVE_OFF 0xd
+#define DRV260X_SUSTAIN_P_OFF 0xe
+#define DRV260X_SUSTAIN_N_OFF 0xf
+#define DRV260X_BRAKE_OFF 0x10
+#define DRV260X_A_TO_V_CTRL 0x11
+#define DRV260X_A_TO_V_MIN_INPUT 0x12
+#define DRV260X_A_TO_V_MAX_INPUT 0x13
+#define DRV260X_A_TO_V_MIN_OUT 0x14
+#define DRV260X_A_TO_V_MAX_OUT 0x15
+#define DRV260X_RATED_VOLT 0x16
+#define DRV260X_OD_CLAMP_VOLT 0x17
+#define DRV260X_CAL_COMP 0x18
+#define DRV260X_CAL_BACK_EMF 0x19
+#define DRV260X_FEEDBACK_CTRL 0x1a
+#define DRV260X_CTRL1 0x1b
+#define DRV260X_CTRL2 0x1c
+#define DRV260X_CTRL3 0x1d
+#define DRV260X_CTRL4 0x1e
+#define DRV260X_CTRL5 0x1f
+#define DRV260X_LRA_LOOP_PERIOD 0x20
+#define DRV260X_VBAT_MON 0x21
+#define DRV260X_LRA_RES_PERIOD 0x22
+#define DRV260X_MAX_REG 0x23
+
+#define DRV260X_ALLOWED_R_BYTES 25
+#define DRV260X_ALLOWED_W_BYTES 2
+#define DRV260X_MAX_RW_RETRIES 5
+#define DRV260X_I2C_RETRY_DELAY 10
+
+#define DRV260X_GO_BIT 0x01
+
+/* Library Selection */
+#define DRV260X_LIB_SEL_MASK 0x07
+#define DRV260X_LIB_SEL_RAM 0x0
+#define DRV260X_LIB_SEL_OD 0x1
+#define DRV260X_LIB_SEL_40_60 0x2
+#define DRV260X_LIB_SEL_60_80 0x3
+#define DRV260X_LIB_SEL_100_140 0x4
+#define DRV260X_LIB_SEL_140_PLUS 0x5
+
+#define DRV260X_LIB_SEL_HIZ_MASK 0x10
+#define DRV260X_LIB_SEL_HIZ_EN 0x01
+#define DRV260X_LIB_SEL_HIZ_DIS 0
+
+/* Mode register */
+#define DRV260X_STANDBY (1 << 6)
+#define DRV260X_STANDBY_MASK 0x40
+#define DRV260X_INTERNAL_TRIGGER 0x00
+#define DRV260X_EXT_TRIGGER_EDGE 0x01
+#define DRV260X_EXT_TRIGGER_LEVEL 0x02
+#define DRV260X_PWM_ANALOG_IN 0x03
+#define DRV260X_AUDIOHAPTIC 0x04
+#define DRV260X_RT_PLAYBACK 0x05
+#define DRV260X_DIAGNOSTICS 0x06
+#define DRV260X_AUTO_CAL 0x07
+
+/* Audio to Haptics Control */
+#define DRV260X_AUDIO_HAPTICS_PEAK_10MS (0 << 2)
+#define DRV260X_AUDIO_HAPTICS_PEAK_20MS (1 << 2)
+#define DRV260X_AUDIO_HAPTICS_PEAK_30MS (2 << 2)
+#define DRV260X_AUDIO_HAPTICS_PEAK_40MS (3 << 2)
+
+#define DRV260X_AUDIO_HAPTICS_FILTER_100HZ 0x00
+#define DRV260X_AUDIO_HAPTICS_FILTER_125HZ 0x01
+#define DRV260X_AUDIO_HAPTICS_FILTER_150HZ 0x02
+#define DRV260X_AUDIO_HAPTICS_FILTER_200HZ 0x03
+
+/* Min/Max Input/Output Voltages */
+#define DRV260X_AUDIO_HAPTICS_MIN_IN_VOLT 0x19
+#define DRV260X_AUDIO_HAPTICS_MAX_IN_VOLT 0x64
+#define DRV260X_AUDIO_HAPTICS_MIN_OUT_VOLT 0x19
+#define DRV260X_AUDIO_HAPTICS_MAX_OUT_VOLT 0xFF
+
+/* Feedback register */
+#define DRV260X_FB_REG_ERM_MODE 0x7f
+#define DRV260X_FB_REG_LRA_MODE (1 << 7)
+
+#define DRV260X_BRAKE_FACTOR_MASK 0x1f
+#define DRV260X_BRAKE_FACTOR_2X (1 << 0)
+#define DRV260X_BRAKE_FACTOR_3X (2 << 4)
+#define DRV260X_BRAKE_FACTOR_4X (3 << 4)
+#define DRV260X_BRAKE_FACTOR_6X (4 << 4)
+#define DRV260X_BRAKE_FACTOR_8X (5 << 4)
+#define DRV260X_BRAKE_FACTOR_16 (6 << 4)
+#define DRV260X_BRAKE_FACTOR_DIS (7 << 4)
+
+#define DRV260X_LOOP_GAIN_LOW 0xf3
+#define DRV260X_LOOP_GAIN_MED (1 << 2)
+#define DRV260X_LOOP_GAIN_HIGH (2 << 2)
+#define DRV260X_LOOP_GAIN_VERY_HIGH (3 << 2)
+
+#define DRV260X_BEMF_GAIN_0 0xfc
+#define DRV260X_BEMF_GAIN_1 (1 << 0)
+#define DRV260X_BEMF_GAIN_2 (2 << 0)
+#define DRV260X_BEMF_GAIN_3 (3 << 0)
+
+/* Control 1 register */
+#define DRV260X_AC_CPLE_EN (1 << 5)
+#define DRV260X_STARTUP_BOOST (1 << 7)
+
+/* Control 2 register */
+
+#define DRV260X_IDISS_TIME_45 0
+#define DRV260X_IDISS_TIME_75 (1 << 0)
+#define DRV260X_IDISS_TIME_150 (1 << 1)
+#define DRV260X_IDISS_TIME_225 0x03
+
+#define DRV260X_BLANK_TIME_45 (0 << 2)
+#define DRV260X_BLANK_TIME_75 (1 << 2)
+#define DRV260X_BLANK_TIME_150 (2 << 2)
+#define DRV260X_BLANK_TIME_225 (3 << 2)
+
+#define DRV260X_SAMP_TIME_150 (0 << 4)
+#define DRV260X_SAMP_TIME_200 (1 << 4)
+#define DRV260X_SAMP_TIME_250 (2 << 4)
+#define DRV260X_SAMP_TIME_300 (3 << 4)
+
+#define DRV260X_BRAKE_STABILIZER (1 << 6)
+#define DRV260X_UNIDIR_IN (0 << 7)
+#define DRV260X_BIDIR_IN (1 << 7)
+
+/* Control 3 Register */
+#define DRV260X_LRA_OPEN_LOOP (1 << 0)
+#define DRV260X_ANANLOG_IN (1 << 1)
+#define DRV260X_LRA_DRV_MODE (1 << 2)
+#define DRV260X_RTP_UNSIGNED_DATA (1 << 3)
+#define DRV260X_SUPPLY_COMP_DIS (1 << 4)
+#define DRV260X_ERM_OPEN_LOOP (1 << 5)
+#define DRV260X_NG_THRESH_0 (0 << 6)
+#define DRV260X_NG_THRESH_2 (1 << 6)
+#define DRV260X_NG_THRESH_4 (2 << 6)
+#define DRV260X_NG_THRESH_8 (3 << 6)
+
+/* Control 4 Register */
+#define DRV260X_AUTOCAL_TIME_150MS (0 << 4)
+#define DRV260X_AUTOCAL_TIME_250MS (1 << 4)
+#define DRV260X_AUTOCAL_TIME_500MS (2 << 4)
+#define DRV260X_AUTOCAL_TIME_1000MS (3 << 4)
+
+struct drv260x_platform_data {
+ int enable_gpio;
+ int library_selection;
+ int mode;
+ int vib_rated_voltage;
+ int vib_overdrive_voltage;
+};
+
+#endif
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] input: ads7846: Switch to managed version of kzalloc and cleanups
From: Pramod Gurav @ 2014-07-28 17:07 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, linux-kernel@vger.kernel.org, Pramod Gurav,
Guenter Roeck, Paul Gortmaker, Mark Brown, Fabio Estevam
In-Reply-To: <20140728164935.GA8982@core.coreip.homeip.net>
Thanks Dmitry for reviewing.
On Mon, Jul 28, 2014 at 10:19 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Pramod,
>
> On Mon, Jul 28, 2014 at 02:16:55PM +0530, pramod.gurav.etc@gmail.com wrote:
>> From: Pramod Gurav <pramod.gurav@smartplayin.com>
>>
>> This switches memory allocations from kzalloc to devm_kzalloc.
>> This also changes the way return checks were done on failure cases
>> of three allocations. The checks were clubbed together and hence
>> must be done seperately to avoid calling kfree on unallocated memory.
>>
>> Moreover in case of input_allocate_device failure, we were calling
>> input_free_device which is not needed.
>
> But not hurting either - like kfree() and many other "cleanup" functions
> in kernel they are happily accept NULL input (so that cleanup code is
> less branchy).
>>
Great. Good to learn.
>> input device must be released(input_free_device) when ads7846_probe_dt
>> fails hence adds a fix there as well.
>
> That is the real bug, could you send me a patch that fixes just that (by
> jumping to err_free_mem)?
Sure I will.
>
> The rest I'd like to leave as is unless you can convert _all_ resources
> to be managed ones: mixing up 2 styles (manual and automatic release) is
> bound to have issues (like we had with ads7846_probe_dt which assumed
> that since it was releasing its resources automatically the rest would
> be released automatically as well.
I thought I would use a managed input_device allocation in a separate patch.
I will convert all allocations to managed then.
This driver also registers a irq. You suggest to convert it to managed as well?
I was planning to do that?
>
> Thanks.
>
> --
> Dmitry
--
Thanks and Regards
Pramod
^ permalink raw reply
* Re: [PATCH 00/15] atmel_mxt_ts - device tree, bootloader, etc
From: Dmitry Torokhov @ 2014-07-28 17:28 UTC (permalink / raw)
To: Stephen Warren
Cc: Nick Dyer, benson Leung, Yufeng Shen, Daniel Kurtz, Yufeng Shen,
Henrik Rydberg, Joonyoung Shim, Alan Bowens, linux-input,
linux-kernel, Peter Meerwald, Olof Johansson, Sekhar Nori
In-Reply-To: <53D2B8D0.5090000@wwwdotorg.org>
On Fri, Jul 25, 2014 at 02:06:40PM -0600, Stephen Warren wrote:
> On 07/25/2014 08:10 AM, Nick Dyer wrote:
> >On 24/07/14 22:19, Stephen Warren wrote:
> ...
> >>I've uploaded 2 logs to:
> >>
> >>http://avon.wwwdotorg.org/downloads/mxt-logs/
> >>(note there's no directory indexing, so manually add the filenames below to
> >>the URL)
> >>
> >>mxt-save-no-movement.xml
> >>
> >>This is with the whole series applied. Neither mouse movement nor clicks
> >>works. I tried mxt-app --reset and it made no difference to the dump results.
> >>
> >>mxt-save-move-ok-no-clicking.xml
> >>
> >>This is with "Input: atmel_mxt_ts - use deep sleep mode when stopped"
> >>reverted; mouse movement works, but clicking doesn't.
> >
> >Great, this has identified the issue with mouse movement (touch).
> >
> >The config programmed into the NVRAM on your touch controller has the first
> >byte of the T9 touchscreen object set to zero. This is the CTRL byte which
> >enables/disables the touch object and what it reports. It is relying on
> >this to enable the touchscreen on resume:
> >
> >https://github.com/dtor/input/blob/9d8dc3e529/drivers/input/touchscreen/atmel_mxt_ts.c#L2005-L2006
> >
> >My "use deep sleep mode when stopped" patch stops the driver writing to the
> >T9.CTRL byte, so whatever config you have in NVRAM for that byte will be
> >used (ie zero, disabled). Going forward, deep sleep is more generic.
> >Indeed, newer chips do not have T9 at all, or they might be using other
> >touch objects. The deep sleep mode is a lower power state to be in, and is
> >what Atmel recommends.
> >
> >However, it does mean changing the maxtouch cfg - you can write the 0x83 to
> >the first byte of T9 and save it to NVRAM, by doing:
> >
> >mxt-app [device] -W -T9 83
> >mxt-app [device] --backup
>
> If I do that, then both mouse movement and "touch" clicks work:-)
>
> (Dmitry, I guess that means it's fine to go ahead and apply "Input:
> atmel_mxt_ts - use deep sleep mode when stopped".)
OK, I've applied it (but because it now last I had to resolve a few
conflicts so if somebody could take a peek at my next branch that would
be great).
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: ads7846: Switch to managed version of kzalloc and cleanups
From: Dmitry Torokhov @ 2014-07-28 17:31 UTC (permalink / raw)
To: Pramod Gurav
Cc: linux-input, linux-kernel@vger.kernel.org, Pramod Gurav,
Guenter Roeck, Paul Gortmaker, Mark Brown, Fabio Estevam
In-Reply-To: <CAMf-jSmHpRLOOcmmW7qOPbwUv3oCtJMvFcqP2LVObE9qQL+D9g@mail.gmail.com>
On Mon, Jul 28, 2014 at 10:37:18PM +0530, Pramod Gurav wrote:
> Thanks Dmitry for reviewing.
>
> On Mon, Jul 28, 2014 at 10:19 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > Hi Pramod,
> >
> > On Mon, Jul 28, 2014 at 02:16:55PM +0530, pramod.gurav.etc@gmail.com wrote:
> >> From: Pramod Gurav <pramod.gurav@smartplayin.com>
> >>
> >> This switches memory allocations from kzalloc to devm_kzalloc.
> >> This also changes the way return checks were done on failure cases
> >> of three allocations. The checks were clubbed together and hence
> >> must be done seperately to avoid calling kfree on unallocated memory.
> >>
> >> Moreover in case of input_allocate_device failure, we were calling
> >> input_free_device which is not needed.
> >
> > But not hurting either - like kfree() and many other "cleanup" functions
> > in kernel they are happily accept NULL input (so that cleanup code is
> > less branchy).
> >>
> Great. Good to learn.
> >> input device must be released(input_free_device) when ads7846_probe_dt
> >> fails hence adds a fix there as well.
> >
> > That is the real bug, could you send me a patch that fixes just that (by
> > jumping to err_free_mem)?
> Sure I will.
>
> >
> > The rest I'd like to leave as is unless you can convert _all_ resources
> > to be managed ones: mixing up 2 styles (manual and automatic release) is
> > bound to have issues (like we had with ads7846_probe_dt which assumed
> > that since it was releasing its resources automatically the rest would
> > be released automatically as well.
>
> I thought I would use a managed input_device allocation in a separate patch.
> I will convert all allocations to managed then.
>
> This driver also registers a irq. You suggest to convert it to managed as well?
> I was planning to do that?
Yes. And regulators, gpios, sysfs (you might need to have custom action
for that) and chip cleanup. OTOH I do not see much value in these
conversions unless they make the code much more clean.
Thanks,
--
Dmitry
^ permalink raw reply
* Re: [v2] input: drv260x: Add TI drv260x haptics driver
From: Dmitry Torokhov @ 2014-07-28 17:43 UTC (permalink / raw)
To: Dan Murphy
Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1406566403-1436-1-git-send-email-dmurphy-l0cyMroinI0@public.gmane.org>
Hi Dan,
On Mon, Jul 28, 2014 at 11:53:23AM -0500, Dan Murphy wrote:
> Add the TI drv260x haptics/vibrator driver.
> This device uses the input force feedback
> to produce a wave form to driver an
> ERM or LRA actuator device.
>
> The initial driver supports the devices
> real time playback mode. But the device
> has additional wave patterns in ROM.
As it presented the device appears to be a memoryless device, however
you present it to the rest of the system as if it can support playback
of multiple effects simultaneously, which is incorrect.
My guess that you need to engage the memoryless input library to schedule
handling multiple effects for your device, including ramping up, ramping
down, stopping playback when effects runs out, etc.
Thanks.
--
Dmitry
P.S. If you are using devm_ there is devm_input_allocate_dveice().
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v6 3/4] HID: lenovo: Don't call function in condition, show error codes
From: Dmitry Torokhov @ 2014-07-28 17:46 UTC (permalink / raw)
To: Jamie Lentin; +Cc: Jiri Kosina, Antonio Ospite, linux-input, linux-kernel
In-Reply-To: <1406385747-2444-1-git-send-email-jm@lentin.co.uk>
On Sat, Jul 26, 2014 at 03:42:27PM +0100, Jamie Lentin wrote:
> Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
> ---
> drivers/hid/hid-lenovo.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index a56b9e7..f96bf09 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -350,6 +350,7 @@ static int lenovo_probe_tpkbd(struct hid_device *hdev)
> size_t name_sz = strlen(dev_name(dev)) + 16;
> char *name_mute, *name_micmute;
> int i;
> + int ret;
>
> /*
> * Only register extra settings against subdevice where input_mapping
> @@ -368,10 +369,9 @@ static int lenovo_probe_tpkbd(struct hid_device *hdev)
> if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 3, 0, 2))
> return -ENODEV;
>
> - if (sysfs_create_group(&hdev->dev.kobj,
> - &lenovo_attr_group_tpkbd)) {
> - hid_warn(hdev, "Could not create sysfs group\n");
> - }
> + ret = sysfs_create_group(&hdev->dev.kobj, &lenovo_attr_group_tpkbd);
> + if (ret)
> + hid_warn(hdev, "Could not create sysfs group: %d\n", ret);
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 1/2] input: wacom: Support up to 2048 pressure levels with ISDv4
From: Dmitry Torokhov @ 2014-07-28 18:00 UTC (permalink / raw)
To: Jason Gerecke; +Cc: Linux Input, Ping Cheng
In-Reply-To: <CANRwn3TSsZMvGU1Oj1iu2a24JKxXfa2fsk8236fj7ek7TgEWeg@mail.gmail.com>
On Fri, Jul 18, 2014 at 04:12:21PM -0700, Jason Gerecke wrote:
> On Mon, Jun 30, 2014 at 2:46 PM, Jason Gerecke <killertofu@gmail.com> wrote:
> > Signed-off-by: Jason Gerecke <killertofu@gmail.com>
> > ---
> > drivers/input/tablet/wacom_wac.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
> > index 977d05c..61a45a5 100644
> > --- a/drivers/input/tablet/wacom_wac.c
> > +++ b/drivers/input/tablet/wacom_wac.c
> > @@ -1093,7 +1093,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
> > input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
> > input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
> > input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
> > - input_report_abs(input, ABS_PRESSURE, ((data[7] & 0x03) << 8) | data[6]);
> > + input_report_abs(input, ABS_PRESSURE, ((data[7] & 0x07) << 8) | data[6]);
> > input_report_key(input, BTN_TOUCH, data[1] & 0x05);
> > input_report_key(input, wacom->tool[0], prox);
> > return 1;
> > --
> > 2.0.0
> >
>
> Just making sure these two patches don't get lost in all the work
> Benjamin's doing. They're going to conflict, so depending on the
> timeline for merging his patches, we can either push these up now(ish)
> or I'll rebase afterwards.
Benjamin added the first one to his series and I adjusted the 2nd one to
apply on top of his and applied.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [v2] input: drv260x: Add TI drv260x haptics driver
From: Murphy, Dan @ 2014-07-28 18:02 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org
In-Reply-To: <20140728174334.GD8982@core.coreip.homeip.net>
Dmitry
Thanks for the comments
On 07/28/2014 12:43 PM, Dmitry Torokhov wrote:
> Hi Dan,
>
> On Mon, Jul 28, 2014 at 11:53:23AM -0500, Dan Murphy wrote:
>> Add the TI drv260x haptics/vibrator driver.
>> This device uses the input force feedback
>> to produce a wave form to driver an
>> ERM or LRA actuator device.
>>
>> The initial driver supports the devices
>> real time playback mode. But the device
>> has additional wave patterns in ROM.
>
> As it presented the device appears to be a memoryless device, however
> you present it to the rest of the system as if it can support playback
> of multiple effects simultaneously, which is incorrect.
>
> My guess that you need to engage the memoryless input library to schedule
> handling multiple effects for your device, including ramping up, ramping
> down, stopping playback when effects runs out, etc.
>
> Thanks.
>
>
I had written the driver originally to be a memless device.
But when calling the driver from user space I was not getting the duration,
strength of any information from the ff_device structure except the type.
The values in the structure were all being passed as 0's.
I will attempt it again as a memless device.
Dan
--
------------------
Dan Murphy
^ 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