* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Christopher Heiny @ 2014-01-09 21:38 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <20140109212949.GA31257@core.coreip.homeip.net>
On 01/09/2014 01:29 PM, Dmitry Torokhov wrote:
> On Thu, Jan 09, 2014 at 01:23:37PM -0800, Christopher Heiny wrote:
>> >On 01/09/2014 12:04 AM, Dmitry Torokhov wrote:
>>> > >On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
>>>> > >>This is a trivial change to replace the sprintf loop with snprintf using
>>>> > >>up-to-date format capability.
>>> > >
>>> > >Hmm, how about we do this instead:
>>> > >
>>> > >Input: synaptics-rmi4 - clean up debug handling in rmi_i2c
>>> > >
>>> > >From: Dmitry Torokhov<dmitry.torokhov@gmail.com>
>>> > >
>>> > >Kernel now has standard facility to format and print binary buffers, let's
>>> > >use it. By doing so we no longer need to allocate memory for debug buffers
>>> > >and we can let debugfs code go as well.
>> >
>> >Not sure where to put this comment, so I'll drop it here.
>> >
>> >I agree the buffers can go. I realized that on the drive home last
>> >night, but was too tired to follow up.
>> >
>> >Talking with some of the folks who use this feature, there's a
>> >desire to keep some sort of finer control on whether the comms
>> >buffers are printed or not - either the existing debugfs setup
>> >(preferred, since it lets them turn on the dmesg clutter only when
>> >needed), or by converting to a config option such as
>> >CONFIG_RMI4_COMMS_DEBUG. It's very useful in new platform
>> >development, since there's a surprising number of ways in which the
>> >reads and writes can go wonky on new hardware.
>
> That is why you have CONFIG_DYNAMIC_DEBUG: you can activate these debug
> statements at will using the common kernel mechanisms.
I'll check this out and get back.
>
> Or we could convert them to dev_vdbg() and then it will be just a tiny
> transport module recompile with DEBUG defined.
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Dmitry Torokhov @ 2014-01-09 21:29 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <52CF1359.6080207@synaptics.com>
On Thu, Jan 09, 2014 at 01:23:37PM -0800, Christopher Heiny wrote:
> On 01/09/2014 12:04 AM, Dmitry Torokhov wrote:
> >On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
> >>This is a trivial change to replace the sprintf loop with snprintf using
> >>up-to-date format capability.
> >
> >Hmm, how about we do this instead:
> >
> >Input: synaptics-rmi4 - clean up debug handling in rmi_i2c
> >
> >From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >
> >Kernel now has standard facility to format and print binary buffers, let's
> >use it. By doing so we no longer need to allocate memory for debug buffers
> >and we can let debugfs code go as well.
>
> Not sure where to put this comment, so I'll drop it here.
>
> I agree the buffers can go. I realized that on the drive home last
> night, but was too tired to follow up.
>
> Talking with some of the folks who use this feature, there's a
> desire to keep some sort of finer control on whether the comms
> buffers are printed or not - either the existing debugfs setup
> (preferred, since it lets them turn on the dmesg clutter only when
> needed), or by converting to a config option such as
> CONFIG_RMI4_COMMS_DEBUG. It's very useful in new platform
> development, since there's a surprising number of ways in which the
> reads and writes can go wonky on new hardware.
That is why you have CONFIG_DYNAMIC_DEBUG: you can activate these debug
statements at will using the common kernel mechanisms.
Or we could convert them to dev_vdbg() and then it will be just a tiny
transport module recompile with DEBUG defined.
Thanks,
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Christopher Heiny @ 2014-01-09 21:23 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <20140109080454.GA27160@core.coreip.homeip.net>
On 01/09/2014 12:04 AM, Dmitry Torokhov wrote:
> On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
>> This is a trivial change to replace the sprintf loop with snprintf using
>> up-to-date format capability.
>
> Hmm, how about we do this instead:
>
> Input: synaptics-rmi4 - clean up debug handling in rmi_i2c
>
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Kernel now has standard facility to format and print binary buffers, let's
> use it. By doing so we no longer need to allocate memory for debug buffers
> and we can let debugfs code go as well.
Not sure where to put this comment, so I'll drop it here.
I agree the buffers can go. I realized that on the drive home last
night, but was too tired to follow up.
Talking with some of the folks who use this feature, there's a desire to
keep some sort of finer control on whether the comms buffers are printed
or not - either the existing debugfs setup (preferred, since it lets
them turn on the dmesg clutter only when needed), or by converting to a
config option such as CONFIG_RMI4_COMMS_DEBUG. It's very useful in new
platform development, since there's a surprising number of ways in which
the reads and writes can go wonky on new hardware.
> We have a new limitation however - output is limited to the first 64 bytes
> of the buffer. However if we really need to capture larger messages dumping
> them in dmesg is not right interface anyway.
>
> Also clean up extra includes, while we are at it.
That's good, too.
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> drivers/input/rmi4/rmi_i2c.c | 131 +++++-------------------------------------
> 1 file changed, 15 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
> index b33074c..c2ad1aa 100644
> --- a/drivers/input/rmi4/rmi_i2c.c
> +++ b/drivers/input/rmi4/rmi_i2c.c
> @@ -8,16 +8,10 @@
> */
>
> #include <linux/kernel.h>
> -#include <linux/delay.h>
> #include <linux/i2c.h>
> -#include <linux/interrupt.h>
> -#include <linux/kconfig.h>
> -#include <linux/lockdep.h>
> #include <linux/module.h>
> -#include <linux/pm.h>
> #include <linux/rmi.h>
> #include <linux/slab.h>
> -#include <linux/uaccess.h>
> #include "rmi_driver.h"
>
> #define BUFFER_SIZE_INCREMENT 32
> @@ -31,11 +25,6 @@
> *
> * @tx_buf: Buffer used for transmitting data to the sensor over i2c.
> * @tx_buf_size: Size of the buffer
> - * @debug_buf: Buffer used for exposing buffer contents using dev_dbg
> - * @debug_buf_size: Size of the debug buffer.
> - *
> - * @comms_debug: Latest data read/written for debugging I2C communications
> - * @debugfs_comms: Debugfs file for debugging I2C communications
> */
> struct rmi_i2c_data {
> struct mutex page_mutex;
> @@ -44,56 +33,8 @@ struct rmi_i2c_data {
>
> u8 *tx_buf;
> int tx_buf_size;
> - u8 *debug_buf;
> - int debug_buf_size;
> -
> - u32 comms_debug;
> -#ifdef CONFIG_RMI4_DEBUG
> - struct dentry *debugfs_comms;
> -#endif
> };
>
> -#ifdef CONFIG_RMI4_DEBUG
> -
> -static int setup_debugfs(struct rmi_device *rmi_dev, struct rmi_i2c_data *data)
> -{
> - if (!rmi_dev->debugfs_root)
> - return -ENODEV;
> -
> - data->debugfs_comms = debugfs_create_bool("comms_debug",
> - (S_IRUGO | S_IWUGO), rmi_dev->debugfs_root,
> - &data->comms_debug);
> - if (!data->debugfs_comms || IS_ERR(data->debugfs_comms)) {
> - dev_warn(&rmi_dev->dev,
> - "Failed to create debugfs comms_debug.\n");
> - data->debugfs_comms = NULL;
> - }
> -
> - return 0;
> -}
> -
> -static void teardown_debugfs(struct rmi_i2c_data *data)
> -{
> - if (data->debugfs_comms)
> - debugfs_remove(data->debugfs_comms);
> -}
> -
> -#else
> -
> -static inline int setup_debugfs(struct rmi_device *rmi_dev,
> - struct rmi_i2c_data *data)
> -{
> - return 0;
> -}
> -
> -static inline void teardown_debugfs(struct rmi_i2c_data *data)
> -{
> -}
> -
> -#endif
> -
> -#define COMMS_DEBUG(data) (IS_ENABLED(CONFIG_RMI4_DEBUG) && data->comms_debug)
> -
> #define RMI_PAGE_SELECT_REGISTER 0xff
> #define RMI_I2C_PAGE(addr) (((addr) >> 8) & 0xff)
>
> @@ -120,8 +61,7 @@ static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
> u8 txbuf[2] = {RMI_PAGE_SELECT_REGISTER, page};
> int retval;
>
> - if (COMMS_DEBUG(data))
> - dev_dbg(&client->dev, "writes 3 bytes: %02x %02x\n",
> + dev_dbg(&client->dev, "writes 3 bytes: %02x %02x\n",
> txbuf[0], txbuf[1]);
> xport->info.tx_count++;
> xport->info.tx_bytes += sizeof(txbuf);
> @@ -136,34 +76,6 @@ static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
> return 0;
> }
>
> -static int copy_to_debug_buf(struct device *dev, struct rmi_i2c_data *data,
> - const u8 *buf, const int len) {
> - int i;
> - int n = 0;
> - char *temp;
> - int dbg_size = 3 * len + 1;
> -
> - if (!data->debug_buf || data->debug_buf_size < dbg_size) {
> - if (data->debug_buf)
> - devm_kfree(dev, data->debug_buf);
> - data->debug_buf_size = dbg_size + BUFFER_SIZE_INCREMENT;
> - data->debug_buf = devm_kzalloc(dev, data->debug_buf_size,
> - GFP_KERNEL);
> - if (!data->debug_buf) {
> - data->debug_buf_size = 0;
> - return -ENOMEM;
> - }
> - }
> - temp = data->debug_buf;
> -
> - for (i = 0; i < len; i++) {
> - n = sprintf(temp, " %02x", buf[i]);
> - temp += n;
> - }
> -
> - return 0;
> -}
> -
> static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
> const void *buf, const int len)
> {
> @@ -195,12 +107,8 @@ static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
> goto exit;
> }
>
> - if (COMMS_DEBUG(data)) {
> - int rc = copy_to_debug_buf(&client->dev, data, (u8 *) buf, len);
> - if (!rc)
> - dev_dbg(&client->dev, "writes %d bytes at %#06x:%s\n",
> - len, addr, data->debug_buf);
> - }
> + dev_dbg(&client->dev,
> + "writes %d bytes at %#06x: %*ph\n", len, addr, len, buf);
>
> xport->info.tx_count++;
> xport->info.tx_bytes += tx_size;
> @@ -232,8 +140,7 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
> goto exit;
> }
>
> - if (COMMS_DEBUG(data))
> - dev_dbg(&client->dev, "writes 1 bytes: %02x\n", txbuf[0]);
> + dev_dbg(&client->dev, "writes 1 bytes: %02x\n", txbuf[0]);
>
> xport->info.tx_count++;
> xport->info.tx_bytes += sizeof(txbuf);
> @@ -244,18 +151,16 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
> goto exit;
> }
>
> - retval = i2c_master_recv(client, (u8 *) buf, len);
> + retval = i2c_master_recv(client, buf, len);
>
> xport->info.rx_count++;
> xport->info.rx_bytes += len;
> if (retval < 0)
> xport->info.rx_errs++;
> - else if (COMMS_DEBUG(data)) {
> - int rc = copy_to_debug_buf(&client->dev, data, (u8 *) buf, len);
> - if (!rc)
> - dev_dbg(&client->dev, "read %d bytes at %#06x:%s\n",
> - len, addr, data->debug_buf);
> - }
> + else
> + dev_dbg(&client->dev,
> + "read %d bytes at %#06x: %*ph\n",
> + len, addr, len, buf);
>
> exit:
> mutex_unlock(&data->page_mutex);
> @@ -265,9 +170,9 @@ exit:
> static int rmi_i2c_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> + const struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
> struct rmi_transport_dev *xport;
> struct rmi_i2c_data *data;
> - struct rmi_device_platform_data *pdata = client->dev.platform_data;
> int retval;
>
> if (!pdata) {
> @@ -318,7 +223,8 @@ static int rmi_i2c_probe(struct i2c_client *client,
>
> mutex_init(&data->page_mutex);
>
> - /* Setting the page to zero will (a) make sure the PSR is in a
> + /*
> + * Setting the page to zero will (a) make sure the PSR is in a
> * known state, and (b) make sure we can talk to the device.
> */
> retval = rmi_set_page(xport, 0);
> @@ -335,11 +241,6 @@ static int rmi_i2c_probe(struct i2c_client *client,
> }
> i2c_set_clientdata(client, xport);
>
> - retval = setup_debugfs(xport->rmi_dev, data);
> - if (retval < 0)
> - dev_warn(&client->dev, "Failed to setup debugfs. Code: %d.\n",
> - retval);
> -
> dev_info(&client->dev, "registered rmi i2c driver at %#04x.\n",
> client->addr);
> return 0;
> @@ -353,14 +254,12 @@ err_gpio:
> static int rmi_i2c_remove(struct i2c_client *client)
> {
> struct rmi_transport_dev *xport = i2c_get_clientdata(client);
> - struct rmi_device_platform_data *pd = client->dev.platform_data;
> -
> - teardown_debugfs(xport->data);
> + struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
>
> rmi_unregister_transport_device(xport);
>
> - if (pd->gpio_config)
> - pd->gpio_config(&pd->gpio_data, false);
> + if (pdata->gpio_config)
> + pdata->gpio_config(&pdata->gpio_data, false);
>
> return 0;
> }
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* Re: [PATCH 0/5] HID logitech DJ fixes and preps for enabling extended features
From: Benjamin Tissoires @ 2014-01-09 21:22 UTC (permalink / raw)
To: Andrew de los Reyes
Cc: Benjamin Tissoires, Jiri Kosina, Nestor Lopez Casado,
Andrew de los Reyes, Linux Input, linux-kernel
In-Reply-To: <CAG_cf+eYoK--k=z+f5xQBx0FP3_z5sbszrSZ1qYq7jR_Zi9Vaw@mail.gmail.com>
On 09/01/14 16:08, Andrew de los Reyes wrote:
> In general, I'm positive on the change to fix the USB3 issue (yay!),
> and for the others I'm happy it's going upstream. It seem to open up
> the possibility of user-space drivers, which is great, even though we
> don't need this on our team.
>
> One thing I want to double-check: on some devices (T651, at least),
> the raw data comes in not via HID++, but tacked onto the end of the
> normal mouse reports. Will a driver for this device be able to get all
> packets, not just HID++ ones? Sorry if this was clear and I missed it.
Yeah, that is partly why I can not send the rest of the series right
now. Nestor already warned me about those funny devices, so I need to
double check how to implement HID++.
On a technical point of view, a driver connected through the unifying
receiver currently only get the regular input reports, and this series
adds the HID++ reports to these ones. So yes, a device will receive all
reports dedicated to it.
Cheers,
Benjamin
>
> -andrew
>
> On Wed, Jan 8, 2014 at 2:18 PM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>> Hi Jiri,
>>
>> Well, this work is _not_ for 3.14 (except maybe patch 1), especially since it is
>> missing the biggest part where we enable the capabilities of Logitech devices.
>>
>> Long story short:
>> This work is based on the work I did back in Summer 2011. I worked at Logitech
>> for a few weeks to show up a demo of a driver for the Logitech Wireless Touchpad.
>> At that time, a first draft has been done, but due to a lack of resources, noone
>> upstreamed it.
>> Since then, the code marinated a little at Logitech and in the ChromeOS kernel
>> tree, but nobody tried to push it upstream. So here I am, trying to push this stuff
>> upstream.
>>
>> I can not send the full series right now because I am lacking most of the
>> testing hardware (I mean I only have the oldest Wireless Touchpad).
>> Hopefully, I should receive some other soon, and I'll be able to send the second
>> part then.
>>
>> Now, let me roughly explain the patches:
>> - patch 1 can be applied right now I think, but it's entirely up to you Jiri.
>> This patch should fix the missing notifications with some USB 3.0 boards.
>> - patches 2 to 5 allows to forward in both direction the proprietary protocol
>> used by Logitech (HID++ [1]) between the driver and the hardware.
>> - later patches will introduce a transport layer for HID++ and also a driver
>> to support full multitouch on various Logitech touchpads.
>>
>> Nestor, Andrew, feel free to add your "Signed-off-by" whereever you want, I lost
>> a little bit the track of who added what.
>>
>> Cheers,
>> Benjamin
>>
>> [1] HID++: Documentation is provided at
>> https://drive.google.com/a/logitech.com/?tab=mo#folders/0BxbRzx7vEV7eWmgwazJ3NUFfQ28
>>
>> Benjamin Tisssoires (5):
>> HID: logitech-dj: Fix USB 3.0 issue
>> HID: core: do not scan reports if the group is already set
>> HID: logitech-dj: rely on hid groups to separate receivers from dj
>> devices
>> HID: logitech-dj: forward incoming HID++ reports to the correct dj
>> device
>> HID: logitech-dj: add .request callback
>>
>> drivers/hid/hid-core.c | 3 +-
>> drivers/hid/hid-logitech-dj.c | 161 +++++++++++++++++++++++++++++++++---------
>> drivers/hid/hid-logitech-dj.h | 16 ++---
>> include/linux/hid.h | 1 +
>> 4 files changed, 136 insertions(+), 45 deletions(-)
>>
>> --
>> 1.8.4.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 3/3] Input: zforce: add devicetree support
From: Heiko Stübner @ 2014-01-09 21:22 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Henrik Rydberg, linux-input
In-Reply-To: <2587208.XYbbI8xJiH@phil>
This makes the zforce driver usable on devicetree-based platforms too.
Signed-off-by: Heiko Stuebner <heiko.stuebner@bqreaders.com>
---
drivers/input/touchscreen/zforce_ts.c | 57 +++++++++++++++++++++++++++++++--
1 file changed, 55 insertions(+), 2 deletions(-)
diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index 5a1a93b..a33e173 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -29,6 +29,8 @@
#include <linux/sysfs.h>
#include <linux/input/mt.h>
#include <linux/platform_data/zforce_ts.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
#define WAIT_TIMEOUT msecs_to_jiffies(1000)
@@ -684,6 +686,45 @@ static void zforce_reset(void *data)
gpio_set_value(ts->pdata->gpio_rst, 0);
}
+static struct zforce_ts_platdata *zforce_parse_dt(struct device *dev)
+{
+ struct zforce_ts_platdata *pdata;
+ struct device_node *np = dev->of_node;
+
+ if (!np)
+ return ERR_PTR(-ENOENT);
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata) {
+ dev_err(dev, "failed to allocate platform data\n");
+ return ERR_PTR(-ENOMEM);
+ }
+
+ pdata->gpio_int = of_get_gpio(np, 0);
+ if (!gpio_is_valid(pdata->gpio_int)) {
+ dev_err(dev, "failed to get interrupt gpio\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ pdata->gpio_rst = of_get_gpio(np, 1);
+ if (!gpio_is_valid(pdata->gpio_rst)) {
+ dev_err(dev, "failed to get reset gpio\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (of_property_read_u32(np, "x-size", &pdata->x_max)) {
+ dev_err(dev, "failed to get x-size property\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (of_property_read_u32(np, "y-size", &pdata->y_max)) {
+ dev_err(dev, "failed to get y-size property\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ return pdata;
+}
+
static int zforce_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -692,8 +733,11 @@ static int zforce_probe(struct i2c_client *client,
struct input_dev *input_dev;
int ret;
- if (!pdata)
- return -EINVAL;
+ if (!pdata) {
+ pdata = zforce_parse_dt(&client->dev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ }
ts = devm_kzalloc(&client->dev, sizeof(struct zforce_ts), GFP_KERNEL);
if (!ts)
@@ -829,11 +873,20 @@ static struct i2c_device_id zforce_idtable[] = {
};
MODULE_DEVICE_TABLE(i2c, zforce_idtable);
+#ifdef CONFIG_OF
+static struct of_device_id zforce_dt_idtable[] = {
+ { .compatible = "neonode,zforce" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, zforce_dt_idtable);
+#endif
+
static struct i2c_driver zforce_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "zforce-ts",
.pm = &zforce_pm_ops,
+ .of_match_table = of_match_ptr(zforce_dt_idtable),
},
.probe = zforce_probe,
.id_table = zforce_idtable,
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/3] Input: zforce: Use internal pdata pointer instead of dev_get_platdata
From: Heiko Stübner @ 2014-01-09 21:21 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Henrik Rydberg, linux-input
In-Reply-To: <2587208.XYbbI8xJiH@phil>
When the device is initialized from devicetree the platformdata is created
locally making dev_get_platdata return NULL.
Therefore directly use the internal pointer to the pdata instead.
Signed-off-by: Heiko Stuebner <heiko.stuebner@bqreaders.com>
---
drivers/input/touchscreen/zforce_ts.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/input/touchscreen/zforce_ts.c b/drivers/input/touchscreen/zforce_ts.c
index 2175f34..5a1a93b 100644
--- a/drivers/input/touchscreen/zforce_ts.c
+++ b/drivers/input/touchscreen/zforce_ts.c
@@ -255,7 +255,7 @@ static int zforce_setconfig(struct zforce_ts *ts, char b1)
static int zforce_start(struct zforce_ts *ts)
{
struct i2c_client *client = ts->client;
- const struct zforce_ts_platdata *pdata = dev_get_platdata(&client->dev);
+ const struct zforce_ts_platdata *pdata = ts->pdata;
int ret;
dev_dbg(&client->dev, "starting device\n");
@@ -326,7 +326,7 @@ static int zforce_stop(struct zforce_ts *ts)
static int zforce_touch_event(struct zforce_ts *ts, u8 *payload)
{
struct i2c_client *client = ts->client;
- const struct zforce_ts_platdata *pdata = dev_get_platdata(&client->dev);
+ const struct zforce_ts_platdata *pdata = ts->pdata;
struct zforce_point point;
int count, i, num = 0;
@@ -471,7 +471,7 @@ static irqreturn_t zforce_irq_thread(int irq, void *dev_id)
{
struct zforce_ts *ts = dev_id;
struct i2c_client *client = ts->client;
- const struct zforce_ts_platdata *pdata = dev_get_platdata(&client->dev);
+ const struct zforce_ts_platdata *pdata = ts->pdata;
int ret;
u8 payload_buffer[512];
u8 *payload;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/3] dt-bindings: bindings for zforce touchscreens
From: Heiko Stübner @ 2014-01-09 21:21 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Henrik Rydberg, linux-input, Rob Herring, Pawel Moll,
Mark Rutland, Stephen Warren, Ian Campbell, devicetree
In-Reply-To: <2587208.XYbbI8xJiH@phil>
Binding documentation for Neonode zForce infrared touchscreens.
Signed-off-by: Heiko Stuebner <heiko.stuebner@bqreaders.com>
---
.../bindings/input/touchscreen/zforce_ts.txt | 30 ++++++++++++++++++++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
2 files changed, 31 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
diff --git a/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
new file mode 100644
index 0000000..2f25cd2
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
@@ -0,0 +1,30 @@
+* Neonode infrared touchscreen controller
+
+Required properties:
+- compatible: must be "neonode,zforce"
+- reg: I2C address of the chip
+- interrupts: interrupt to which the chip is connected
+- gpios: gpios the chip is connected to
+ first one is the interrupt gpio and second one the reset gpio
+- x-size: horizontal resolution of touchscreen
+- y-size: vertical resolution of touchscreen
+
+Example:
+
+ i2c@00000000 {
+ /* ... */
+
+ zforce_ts@50 {
+ compatible = "neonode,zforce";
+ reg = <0x50>;
+ interrupts = <2 0>;
+
+ gpios = <&gpio5 6 0>, /* INT */
+ <&gpio5 9 0>; /* RST */
+
+ x-size = <800>;
+ y-size = <600>;
+ };
+
+ /* ... */
+ };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index b458760..e0b15b4 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -53,6 +53,7 @@ maxim Maxim Integrated Products
microchip Microchip Technology Inc.
mosaixtech Mosaix Technologies, Inc.
national National Semiconductor
+neonode Neonode Inc.
nintendo Nintendo
nvidia NVIDIA
nxp NXP Semiconductors
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/3] Input: add dt support to zforce driver
From: Heiko Stübner @ 2014-01-09 21:19 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Henrik Rydberg, linux-input
This adds the binding documentation and necessary parsing code to make
zforce based touchscreen usable on devicetree platforms.
Heiko Stuebner (3):
dt-bindings: bindings for zforce touchscreens
Input: zforce: Use internal pdata pointer instead of dev_get_platdata
Input: zforce: add devicetree support
.../bindings/input/touchscreen/zforce_ts.txt | 30 ++++++++++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/input/touchscreen/zforce_ts.c | 63 ++++++++++++++++++--
3 files changed, 89 insertions(+), 5 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/zforce_ts.txt
--
1.7.10.4
^ permalink raw reply
* Re: [PATCH 0/5] HID logitech DJ fixes and preps for enabling extended features
From: Andrew de los Reyes @ 2014-01-09 21:08 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Jiri Kosina, Nestor Lopez Casado,
Andrew de los Reyes, Linux Input, linux-kernel
In-Reply-To: <1389219529-29671-1-git-send-email-benjamin.tissoires@redhat.com>
In general, I'm positive on the change to fix the USB3 issue (yay!),
and for the others I'm happy it's going upstream. It seem to open up
the possibility of user-space drivers, which is great, even though we
don't need this on our team.
One thing I want to double-check: on some devices (T651, at least),
the raw data comes in not via HID++, but tacked onto the end of the
normal mouse reports. Will a driver for this device be able to get all
packets, not just HID++ ones? Sorry if this was clear and I missed it.
-andrew
On Wed, Jan 8, 2014 at 2:18 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> Hi Jiri,
>
> Well, this work is _not_ for 3.14 (except maybe patch 1), especially since it is
> missing the biggest part where we enable the capabilities of Logitech devices.
>
> Long story short:
> This work is based on the work I did back in Summer 2011. I worked at Logitech
> for a few weeks to show up a demo of a driver for the Logitech Wireless Touchpad.
> At that time, a first draft has been done, but due to a lack of resources, noone
> upstreamed it.
> Since then, the code marinated a little at Logitech and in the ChromeOS kernel
> tree, but nobody tried to push it upstream. So here I am, trying to push this stuff
> upstream.
>
> I can not send the full series right now because I am lacking most of the
> testing hardware (I mean I only have the oldest Wireless Touchpad).
> Hopefully, I should receive some other soon, and I'll be able to send the second
> part then.
>
> Now, let me roughly explain the patches:
> - patch 1 can be applied right now I think, but it's entirely up to you Jiri.
> This patch should fix the missing notifications with some USB 3.0 boards.
> - patches 2 to 5 allows to forward in both direction the proprietary protocol
> used by Logitech (HID++ [1]) between the driver and the hardware.
> - later patches will introduce a transport layer for HID++ and also a driver
> to support full multitouch on various Logitech touchpads.
>
> Nestor, Andrew, feel free to add your "Signed-off-by" whereever you want, I lost
> a little bit the track of who added what.
>
> Cheers,
> Benjamin
>
> [1] HID++: Documentation is provided at
> https://drive.google.com/a/logitech.com/?tab=mo#folders/0BxbRzx7vEV7eWmgwazJ3NUFfQ28
>
> Benjamin Tisssoires (5):
> HID: logitech-dj: Fix USB 3.0 issue
> HID: core: do not scan reports if the group is already set
> HID: logitech-dj: rely on hid groups to separate receivers from dj
> devices
> HID: logitech-dj: forward incoming HID++ reports to the correct dj
> device
> HID: logitech-dj: add .request callback
>
> drivers/hid/hid-core.c | 3 +-
> drivers/hid/hid-logitech-dj.c | 161 +++++++++++++++++++++++++++++++++---------
> drivers/hid/hid-logitech-dj.h | 16 ++---
> include/linux/hid.h | 1 +
> 4 files changed, 136 insertions(+), 45 deletions(-)
>
> --
> 1.8.4.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Christopher Heiny @ 2014-01-09 21:02 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <cdba3581-ff07-4f3e-900e-2ee5c6e404c8@email.android.com>
On 01/09/2014 12:48 PM, Dmitry Torokhov wrote:
> Christopher Heiny<cheiny@synaptics.com> wrote:
>> >On 01/09/2014 12:28 AM, Dmitry Torokhov wrote:
>>> >>On Thu, Jan 09, 2014 at 12:04:54AM -0800, Dmitry Torokhov wrote:
>>>> >>>On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
>>>>> >>>>This is a trivial change to replace the sprintf loop with snprintf
>> >using
>>>>> >>>>up-to-date format capability.
>>>> >>>
>>>> >>>Hmm, how about we do this instead:
>>> >>
>>> >>And another small one...
>>> >>
>>> >>Input: synaptics-rmi4 - transport name should be a const pointer
>>> >>
>>> >>From: Dmitry Torokhov<dmitry.torokhov@gmail.com>
>>> >>
>>> >>Signed-off-by: Dmitry Torokhov<dmitry.torokhov@gmail.com>
>> >
>> >Looks good to me. I think the correct protocol is to Ack the patch,
>> >so...
>> >
>> >Acked-by: Christopher Heiny<cheiny@synaptics.com>
> Both or only the const name one?
>
The const name one only at this point. I have a couple of comments on
the debug output, one.
^ permalink raw reply
* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Dmitry Torokhov @ 2014-01-09 20:48 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <52CF06BA.5040000@synaptics.com>
Christopher Heiny <cheiny@synaptics.com> wrote:
>On 01/09/2014 12:28 AM, Dmitry Torokhov wrote:
>> On Thu, Jan 09, 2014 at 12:04:54AM -0800, Dmitry Torokhov wrote:
>>> On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
>>>> This is a trivial change to replace the sprintf loop with snprintf
>using
>>>> up-to-date format capability.
>>>
>>> Hmm, how about we do this instead:
>>
>> And another small one...
>>
>> Input: synaptics-rmi4 - transport name should be a const pointer
>>
>> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>
>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
>Looks good to me. I think the correct protocol is to Ack the patch,
>so...
>
>Acked-by: Christopher Heiny <cheiny@synaptics.com>
Both or only the const name one?
>
>
>> ---
>> drivers/input/rmi4/rmi_bus.h | 2 +-
>> drivers/input/rmi4/rmi_i2c.c | 4 +---
>> 2 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/input/rmi4/rmi_bus.h
>b/drivers/input/rmi4/rmi_bus.h
>> index 65dd934..3e8b57a 100644
>> --- a/drivers/input/rmi4/rmi_bus.h
>> +++ b/drivers/input/rmi4/rmi_bus.h
>> @@ -148,7 +148,7 @@ struct rmi_driver {
>> * @att_count Number of times ATTN assertions have been handled.
>> */
>> struct rmi_transport_info {
>> - char *proto;
>> + const char *proto;
>> long tx_count;
>> long tx_bytes;
>> long tx_errs;
>> diff --git a/drivers/input/rmi4/rmi_i2c.c
>b/drivers/input/rmi4/rmi_i2c.c
>> index ea01823..ebe74ec 100644
>> --- a/drivers/input/rmi4/rmi_i2c.c
>> +++ b/drivers/input/rmi4/rmi_i2c.c
>> @@ -38,8 +38,6 @@ struct rmi_i2c_data {
>> #define RMI_PAGE_SELECT_REGISTER 0xff
>> #define RMI_I2C_PAGE(addr) (((addr) >> 8) & 0xff)
>>
>> -static char *xport_proto_name = "i2c";
>> -
>> /*
>> * rmi_set_page - Set RMI page
>> * @xport: The pointer to the rmi_transport_dev struct
>> @@ -217,7 +215,7 @@ static int rmi_i2c_probe(struct i2c_client
>*client,
>>
>> xport->write_block = rmi_i2c_write_block;
>> xport->read_block = rmi_i2c_read_block;
>> - xport->info.proto = xport_proto_name;
>> + xport->info.proto = "i2c";
>>
>> mutex_init(&data->page_mutex);
>>
>>
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Christopher Heiny @ 2014-01-09 20:29 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <20140109082858.GE27160@core.coreip.homeip.net>
On 01/09/2014 12:28 AM, Dmitry Torokhov wrote:
> On Thu, Jan 09, 2014 at 12:04:54AM -0800, Dmitry Torokhov wrote:
>> On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
>>> This is a trivial change to replace the sprintf loop with snprintf using
>>> up-to-date format capability.
>>
>> Hmm, how about we do this instead:
>
> And another small one...
>
> Input: synaptics-rmi4 - transport name should be a const pointer
>
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Looks good to me. I think the correct protocol is to Ack the patch, so...
Acked-by: Christopher Heiny <cheiny@synaptics.com>
> ---
> drivers/input/rmi4/rmi_bus.h | 2 +-
> drivers/input/rmi4/rmi_i2c.c | 4 +---
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
> index 65dd934..3e8b57a 100644
> --- a/drivers/input/rmi4/rmi_bus.h
> +++ b/drivers/input/rmi4/rmi_bus.h
> @@ -148,7 +148,7 @@ struct rmi_driver {
> * @att_count Number of times ATTN assertions have been handled.
> */
> struct rmi_transport_info {
> - char *proto;
> + const char *proto;
> long tx_count;
> long tx_bytes;
> long tx_errs;
> diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
> index ea01823..ebe74ec 100644
> --- a/drivers/input/rmi4/rmi_i2c.c
> +++ b/drivers/input/rmi4/rmi_i2c.c
> @@ -38,8 +38,6 @@ struct rmi_i2c_data {
> #define RMI_PAGE_SELECT_REGISTER 0xff
> #define RMI_I2C_PAGE(addr) (((addr) >> 8) & 0xff)
>
> -static char *xport_proto_name = "i2c";
> -
> /*
> * rmi_set_page - Set RMI page
> * @xport: The pointer to the rmi_transport_dev struct
> @@ -217,7 +215,7 @@ static int rmi_i2c_probe(struct i2c_client *client,
>
> xport->write_block = rmi_i2c_write_block;
> xport->read_block = rmi_i2c_read_block;
> - xport->info.proto = xport_proto_name;
> + xport->info.proto = "i2c";
>
> mutex_init(&data->page_mutex);
>
>
--
Christopher Heiny
Senior Staff Firmware Engineer
Synaptics Incorporated
^ permalink raw reply
* [PATCH] input: synaptics-rmi4 - fix F01 DOM formatting
From: Christopher Heiny @ 2014-01-09 20:21 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linux Input, Christopher Heiny, Andrew Duggan, Vincent Huang,
Vivian Ly, Daniel Rosenberg, Jean Delvare, Joerie de Gram,
Linus Walleij, Benjamin Tissoires
Use a sensible format string for the date of manufacture formatting.
Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/input/rmi4/rmi_f01.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 1cb11ea..9e6a578 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -50,7 +50,7 @@ struct f01_basic_properties {
bool has_lts;
bool has_adjustable_doze;
bool has_adjustable_doze_holdoff;
- char dom[9]; /* YYYYMMDD + '\0' */
+ char dom[11]; /* YYYY/MM/DD + '\0' */
u8 product_id[RMI_PRODUCT_ID_LENGTH + 1];
u16 productinfo;
};
@@ -190,8 +190,7 @@ static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
props->has_adjustable_doze_holdoff =
basic_query[1] & RMI_F01_QRY1_HAS_ADJ_DOZE_HOFF;
- snprintf(props->dom, sizeof(props->dom),
- "20%02x%02x%02x",
+ snprintf(props->dom, sizeof(props->dom), "20%02d/%02d/%02d",
basic_query[5] & RMI_F01_QRY5_YEAR_MASK,
basic_query[6] & RMI_F01_QRY6_MONTH_MASK,
basic_query[7] & RMI_F01_QRY7_DAY_MASK);
^ permalink raw reply related
* Re: [PATCH 1/2] HID: sony: Add force feedback for the Dualshock 4
From: Frank Praznik @ 2014-01-09 19:29 UTC (permalink / raw)
To: simon; +Cc: linux-input
In-Reply-To: <9702a6d4a97cca7e0f05dd2329dca326.squirrel@mungewell.org>
On 1/9/2014 01:42, simon@mungewell.org wrote:
>>> However I am seeing some weird behaviour in whether FF/LED actually
>>> functions. It seems that in a 'complete' kernel installation the driver
>>> does not present FF or LED.
>>>
> Found the problem (or at least a solution), there is no entry in 'hid-core.c'
> --
> static const struct hid_device_id hid_have_special_driver[] = {
> ...
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY,
> USB_DEVICE_ID_SONY_PS4_CONTROLLER) },
> --
>
> Adding this makes it work for me, although I don't know why the insmod
> trick was working....
> Simon
>
Ah, I didn't realize that it needed an entry there. I'll add this to v2
of the patch set. Thanks!
^ permalink raw reply
* Re: [PATCH V1] input: use device managed memory in da9052 touchscreen driver
From: Dmitry Torokhov @ 2014-01-09 19:12 UTC (permalink / raw)
To: Anthony Olech; +Cc: linux-input, linux-kernel, Huqiu Liu, David Dajun Chen
In-Reply-To: <201401091252.s09CqsOr042762@swsrvapps-02.lan>
Hi Anthony,
On Thu, Jan 09, 2014 at 12:51:37PM +0000, Anthony Olech wrote:
> The touchscreen component driver for the da9052/3 Dialog PMICs
> is changed to use device managed memory allocation.
>
> This results in simpler error paths as failures in the probe()
> function do not require explicit calls to free the devm_...
> allocated memory.
> The allocation functions used in this driver are:
> devm_kzalloc()
> devm_input_allocate_device()
> devm_request_threaded_irq()
>
> Suggested-by: Huqiu Liu <liuhq11@mails.tsinghua.edu.cn>
> Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com>
> ---
> This patch is relative to linux-next repository tag next-20140109
>
> Many thanks to Huqiu Liu who instigated this patch.
>
> drivers/input/touchscreen/da9052_tsi.c | 62 ++++++++++++++++----------------
> 1 file changed, 30 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/input/touchscreen/da9052_tsi.c b/drivers/input/touchscreen/da9052_tsi.c
> index ab64d58..dcc4cf1 100644
> --- a/drivers/input/touchscreen/da9052_tsi.c
> +++ b/drivers/input/touchscreen/da9052_tsi.c
> @@ -233,18 +233,30 @@ static int da9052_ts_probe(struct platform_device *pdev)
> struct da9052_tsi *tsi;
> struct input_dev *input_dev;
> int error;
> + int pdown_irq;
> + int ready_irq;
>
> da9052 = dev_get_drvdata(pdev->dev.parent);
> if (!da9052)
> return -EINVAL;
>
> - tsi = kzalloc(sizeof(struct da9052_tsi), GFP_KERNEL);
> - input_dev = input_allocate_device();
> - if (!tsi || !input_dev) {
> - error = -ENOMEM;
> - goto err_free_mem;
> - }
> + pdown_irq = regmap_irq_get_virq(da9052->irq_data, DA9052_IRQ_PENDOWN);
> + if (pdown_irq < 0)
> + return pdown_irq;
...
>
> - error = da9052_request_irq(tsi->da9052, DA9052_IRQ_PENDOWN,
> - "pendown-irq", da9052_ts_pendwn_irq, tsi);
> + error = devm_request_threaded_irq(&pdev->dev, pdown_irq,
> + NULL, da9052_ts_pendwn_irq,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + "pendown-irq", tsi);
I am uncomfortable with the touchscreen portion of this driver ignoring
the framework of it's MFD core and mixing native IRQ management with the
ones done through the core.
What would happen if somebody changes da9052_request_irq() to do some
thing more than it is doing now so that your open-coded duplicate of the
same in da9052_ts_probe() is no longer equivalent? Or
da9052_disable_irq() no longer works correctly with IRQs allocated by
this sub-module?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: gamepad - use independent axes for analog D-Pad buttons
From: David Herrmann @ 2014-01-09 14:33 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Antonio Ospite, open list:HID CORE LAYER, Jiri Kosina,
Benjamin Tissoires
In-Reply-To: <20131230185041.GB3278@core.coreip.homeip.net>
Hi Dmitry
On Mon, Dec 30, 2013 at 7:50 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Mon, Dec 30, 2013 at 12:44:21PM +0100, David Herrmann wrote:
>> Hi
>>
>> On Mon, Dec 30, 2013 at 12:20 PM, Antonio Ospite
>> <ospite@studenti.unina.it> wrote:
>> > On Sun, 29 Dec 2013 16:52:09 -0800
>> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>> >
>> >> Hi Antonio,
>> >>
>> >> On Mon, Dec 23, 2013 at 05:17:43PM +0100, Antonio Ospite wrote:
>> >> > Model this part of the API after the Sony PlayStation 3 Controller which
>> >> > exposes independent analog values for each one of the D-Pad buttons.
>> >> >
>> >> > The PS3 programming API psl1ght also maps the analog D-Pad buttons
>> >> > individually.
>> >>
>> >> Hmm, even though the hardware is capable of producing independent analog
>> >> values does are they really useful? Looking at my PS3 controller I do
>> >> not think users will be pressing up/down and left/right dpad buttons at
>> >> the same time.
>> >>
>> >
>> > I must agree it's unlikely, while still possible.
>> >
>> >> I'd rather keep using ABS_HAT0X/Y unless there is really good reason for
>> >> introducing new events.
>> >>
>> >
>> > Having analog D-Pad values reported independently was proposed for these
>> > reasons:
>> >
>> > - it matches _some_ existing hardware closely (that was the main
>> > reason TBH, to simplify the drivers);
>> >
>> > - it happens to follow what it's being done for D-Pad digital buttons,
>> > they are also reported independently.
>> >
>> > However if some other hardware reported D-Pad axis combined already
>> > then I agree that using something like ABS_HAT0X/Y is safer, I see
>> > decomposing HORIZ/VERT into the separate LEFT/RIGHT and UP/DOWN to be
>> > less intuitive (not harder tho).
>> >
>> > Another doubt: David, in the other email you mentioned we could use
>> > ABS_DPAD_HORIZ/VERT, any particular reason to introduce them in place
>> > of ABS_HAT0X/Y?
>>
>> A "HAT" is an axis on the top of a joystick, nothing else. It seems
>> troublesome to overload the definition (which we did for many years).
>> Device-detection based on advertised ABS-bits gets pretty hard if we
>> reuse ABS-bits for different hardware. The most annoying example of
>> what happens is accelerometers being misdetected by Xorg as
>> mouse-input because they reuse ABS_X/Y.
>
> But they are good as joysticks (also ABS_X/ABS_Y). I do not think having
> all unique events for all device types is feasible. Can we provide hints
> (via properties) to lessen ambiguity, like we do with direct/indirect
> pointers for touchscreens/tablets?
Why don't you think it's feasible? For keys we have one definition for
each key, we don't do KEY_0 to KEY_65535 and just use the first few.
I'd really like to see the same for ABS_*. It's troublesome to detect
devices in user-space otherwise. But I guess your concern is rather
about the implementation than the idea. So could you let me know what
exactly makes you think it's not feasible?
The only problem I see is ->absinfo[] getting too big. My solution for
this would be to add a "uint16_t code" to "struct input_absinfo". So
we keep our current limited ABS set and drivers use just these. But to
add semantics, we can define additional/other ABS2_* or ABS_INFO_*
codes which define what axis this exactly is. So the axis-index is
still the limited ABS code, but to get the semantic code we query
input_absinfo.
Adding additional PROP_* bits is imho the wrong way. For instance if
we use ABS_X/Y for absolute touchpad input and the same for an
accelerometer, devices like the steam-controller couldn't report both
in the same device. Even if they set PROP_TOUCHPAD and PROP_GAMEPAD.
I'd like to get this figured out before I send v3 of the ABS2 patches.
Thanks
David
^ permalink raw reply
* Re: [PATCHv6] staging/iio/adc: change the MXS touchscreen driver implementation
From: Alexandre Belloni @ 2014-01-09 13:31 UTC (permalink / raw)
To: Juergen Beisert, linux-iio-u79uwXL29TY76Z2rM5mHXA
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b, marex-ynQEQJNshbs,
fabio.estevam-KZfg59tc24xl57MIdRCFDg,
jic23-KWPb1pKIrIJaa/9Udqfwiw, linux-input-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1379946998-23041-1-git-send-email-jbe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Hi,
Sorry to chime in only now but it seems that this series is breaking the
touchscreen calibration on 3.13 (and -rc7 is out so it might be too
late).
At first, I though I became a terrible clicker ;) but I found some
evidences:
xinput_calibrator is complaining about misclicks, debug output:
DEBUG: Adding click 0 (X=105, Y=59)
DEBUG: Not adding click 1 (X=100, Y=59): within 7 pixels of previous click
DEBUG: Adding click 1 (X=696, Y=58)
DEBUG: Not adding click 2 (X=700, Y=55): within 7 pixels of previous click
DEBUG: Adding click 2 (X=103, Y=422)
DEBUG: Mis-click detected, click 3 (X=438, Y=415) not aligned with click 1 (X=696, Y=58) or click 2 (X=103, Y=422) (threshold=15)
DEBUG: Adding click 0 (X=424, Y=411)
Do you see the confusion ? At some point one of the coordinates is not
updated. Successful calibration with 3.12.6 gives:
DEBUG: Adding click 0 (X=126, Y=405)
DEBUG: Adding click 1 (X=684, Y=399)
DEBUG: Adding click 2 (X=128, Y=77)
DEBUG: Adding click 3 (X=683, Y=77)
With ts_calibrate:
xres = 800, yres = 480
Took 1 samples...
Top left : X = 435 Y = 3572
Took 2 samples...
Top right : X = 2094 Y = 3553
Took 2 samples...
Bot right : X = 2939 Y = 2077
Took 2 samples...
Bot left : X = 2060 Y = 644
Took 2 samples...
Center : X = 1279 Y = 1346
We experience the same thing, when changing position on an axis, we get
an average between the previous and the new position (probably because
we got 2 samples):
- Top Left is ok
- Top right is almost ok: X should be around 3500/3700
- Bot right is starting to get really wrong: X is getting better (it
should still be around 3500/3700) but Y should be quite lower
- Bot left: Y is ok but X should be lower
- Center is completely wrong, both values should be around 2000
Last test with evtest which is always difficult because it outputs a lot
of debug. Two separate touchs, bottom left and then top right (note that
this is with fsl,ave-ctrl = <8>;):
Event: time 1389210862.451000, type 3 (Absolute), code 0 (X), value 2183
Event: time 1389210862.451000, type 3 (Absolute), code 1 (Y), value 720
Event: time 1389210862.451000, type 3 (Absolute), code 24 (Pressure), value 6
Event: time 1389210862.451000, type 1 (Key), code 330 (Touch), value 1
Event: time 1389210862.451000, -------------- Report Sync ------------
Event: time 1389210862.477721, type 3 (Absolute), code 0 (X), value 448
Event: time 1389210862.477721, type 3 (Absolute), code 1 (Y), value 667
Event: time 1389210862.477721, type 3 (Absolute), code 24 (Pressure), value 595
Event: time 1389210862.477721, -------------- Report Sync ------------
Event: time 1389210862.504718, type 3 (Absolute), code 0 (X), value 434
Event: time 1389210862.504718, type 3 (Absolute), code 1 (Y), value 704
Event: time 1389210862.504718, type 3 (Absolute), code 24 (Pressure), value 580
Event: time 1389210862.504718, -------------- Report Sync ------------
Event: time 1389210862.536688, type 1 (Key), code 330 (Touch), value 0
Event: time 1389210862.536688, -------------- Report Sync ------------
Event: time 1389210863.359697, type 3 (Absolute), code 0 (X), value 432
Event: time 1389210863.359697, type 3 (Absolute), code 1 (Y), value 726
Event: time 1389210863.359697, type 3 (Absolute), code 24 (Pressure), value 210
Event: time 1389210863.359697, type 1 (Key), code 330 (Touch), value 1
Event: time 1389210863.359697, -------------- Report Sync ------------
Event: time 1389210863.391687, type 1 (Key), code 330 (Touch), value 0
Event: time 1389210863.391687, -------------- Report Sync ------------
We get 3 reports for bottom left, then pen up, then one report for top
right that is almost exactly the same a bottom left !
Output from 3.12:
Event: time 1389210022.146809, type 3 (Absolute), code 0 (X), value 286
Event: time 1389210022.146809, type 3 (Absolute), code 1 (Y), value 504
Event: time 1389210022.146809, type 3 (Absolute), code 24 (Pressure), value 3045
Event: time 1389210022.146809, type 1 (Key), code 330 (Touch), value 1
Event: time 1389210022.146809, -------------- Report Sync ------------
Event: time 1389210022.166830, type 3 (Absolute), code 0 (X), value 256
Event: time 1389210022.166830, type 3 (Absolute), code 1 (Y), value 489
Event: time 1389210022.166830, type 3 (Absolute), code 24 (Pressure), value 3033
Event: time 1389210022.166830, -------------- Report Sync ------------
Event: time 1389210022.186812, type 3 (Absolute), code 24 (Pressure), value 0
Event: time 1389210022.186812, type 1 (Key), code 330 (Touch), value 0
Event: time 1389210022.186812, -------------- Report Sync ------------
Event: time 1389210025.196808, type 3 (Absolute), code 0 (X), value 3812
Event: time 1389210025.196808, type 3 (Absolute), code 1 (Y), value 3637
Event: time 1389210025.196808, type 3 (Absolute), code 24 (Pressure), value 4032
Event: time 1389210025.196808, type 1 (Key), code 330 (Touch), value 1
Event: time 1389210025.196808, -------------- Report Sync ------------
Event: time 1389210025.216819, type 3 (Absolute), code 24 (Pressure), value 0
Event: time 1389210025.216819, type 1 (Key), code 330 (Touch), value 0
Event: time 1389210025.216819, -------------- Report Sync ------------
While this is not necessarily an issue with ts_calibrate (it is possible
to click longer so it collects more samples), I don't see that working
with xinput_calibrator.
While I don't have much experience with the TS part of the code but I
can investigate if you don't have any idea.
On 23/09/2013 16:36, Juergen Beisert wrote:
> The following series replaces the current busy loop touchscreen implementation
> for i.MX28/i.MX23 SoCs by a fully interrupt driven implementation.
>
> Since i.MX23 and i.MX28 silicon differs, the existing implementation can
> be used for the i.MX28 SoC only.
>
> The first patch adds proper clock handling. Various platforms seems to disable
> the internal 2 kHz clock which is used by the LRADC delay units.
>
> The next two patches of this series move the i.MX28 specific definitions
> out of the way. The forth patch simplifies the register access to make it easier
> to add the i.MX23 support. Then the i.MX23 specific definitions are added, also
> the code to distinguish both SoCs at run-time.
> Up to here the existing touchscreen driver will now run on an i.MX23 Soc as well.
>
> When these i.MX SoCs are running from battery it seems not to be a good idea to
> run a busy loop to detect touches and their location. The 6th patch adds a
> fully interrupt driven implementation which makes use of the built-in delay
> and multiple sample features of the touchscreen controller. This will reduce
> the interrupt load to a minimum.
>
> The remaining patches in this series just removes the existing busy loop
> implementation, add a proposal for devicetree binding and a reminder what has
> still to be done with the LRADC driver.
>
> Changes since v5:
>
> - add missing clock handling which prevents the delay units from work (this
> should make it work on the MX28EVK and M28EVK as well)
>
> Changes since v4:
>
> - honor Jonathan's comments about function names
> - honor Dmitry's comments about workqueue canceling and interrupts
> - adding devicetree bindings proposal
>
> Changes since v3:
>
> - split adding register access functions and i.MX23 support into two patches
>
> Changes since v2:
>
> - useless debug output removed
>
> Changes since v1:
>
> - adding register access functions to make the existing code more readable
> - adding some functions to distinguish the SoCs at run-time to avoid if-else
> contructs whenever differences in the register layout between i.MX23 and
> i.MX28 must be handled
>
> Comments are welcome.
>
> Juergen
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH V1] input: use device managed memory in da9052 touchscreen driver
From: Anthony Olech @ 2014-01-09 12:51 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Huqiu Liu, David Dajun Chen
The touchscreen component driver for the da9052/3 Dialog PMICs
is changed to use device managed memory allocation.
This results in simpler error paths as failures in the probe()
function do not require explicit calls to free the devm_...
allocated memory.
The allocation functions used in this driver are:
devm_kzalloc()
devm_input_allocate_device()
devm_request_threaded_irq()
Suggested-by: Huqiu Liu <liuhq11@mails.tsinghua.edu.cn>
Signed-off-by: Anthony Olech <anthony.olech.opensource@diasemi.com>
---
This patch is relative to linux-next repository tag next-20140109
Many thanks to Huqiu Liu who instigated this patch.
drivers/input/touchscreen/da9052_tsi.c | 62 ++++++++++++++++----------------
1 file changed, 30 insertions(+), 32 deletions(-)
diff --git a/drivers/input/touchscreen/da9052_tsi.c b/drivers/input/touchscreen/da9052_tsi.c
index ab64d58..dcc4cf1 100644
--- a/drivers/input/touchscreen/da9052_tsi.c
+++ b/drivers/input/touchscreen/da9052_tsi.c
@@ -233,18 +233,30 @@ static int da9052_ts_probe(struct platform_device *pdev)
struct da9052_tsi *tsi;
struct input_dev *input_dev;
int error;
+ int pdown_irq;
+ int ready_irq;
da9052 = dev_get_drvdata(pdev->dev.parent);
if (!da9052)
return -EINVAL;
- tsi = kzalloc(sizeof(struct da9052_tsi), GFP_KERNEL);
- input_dev = input_allocate_device();
- if (!tsi || !input_dev) {
- error = -ENOMEM;
- goto err_free_mem;
- }
+ pdown_irq = regmap_irq_get_virq(da9052->irq_data, DA9052_IRQ_PENDOWN);
+ if (pdown_irq < 0)
+ return pdown_irq;
+
+ ready_irq = regmap_irq_get_virq(da9052->irq_data, DA9052_IRQ_TSIREADY);
+ if (ready_irq < 0)
+ return ready_irq;
+
+ tsi = devm_kzalloc(&pdev->dev, sizeof(struct da9052_tsi), GFP_KERNEL);
+ if (!tsi)
+ return -ENOMEM;
+
+ input_dev = devm_input_allocate_device(&pdev->dev);
+ if (!input_dev)
+ return -ENOMEM;
+ platform_set_drvdata(pdev, tsi);
tsi->da9052 = da9052;
tsi->dev = input_dev;
tsi->stopped = true;
@@ -274,20 +286,24 @@ static int da9052_ts_probe(struct platform_device *pdev)
/* Disable ADC */
da9052_ts_adc_toggle(tsi, false);
- error = da9052_request_irq(tsi->da9052, DA9052_IRQ_PENDOWN,
- "pendown-irq", da9052_ts_pendwn_irq, tsi);
+ error = devm_request_threaded_irq(&pdev->dev, pdown_irq,
+ NULL, da9052_ts_pendwn_irq,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ "pendown-irq", tsi);
if (error) {
dev_err(tsi->da9052->dev,
"Failed to register PENDWN IRQ: %d\n", error);
- goto err_free_mem;
+ return error;
}
- error = da9052_request_irq(tsi->da9052, DA9052_IRQ_TSIREADY,
- "tsiready-irq", da9052_ts_datardy_irq, tsi);
+ error = devm_request_threaded_irq(&pdev->dev, ready_irq,
+ NULL, da9052_ts_datardy_irq,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ "tsiready-irq", tsi);
if (error) {
dev_err(tsi->da9052->dev,
"Failed to register TSIRDY IRQ :%d\n", error);
- goto err_free_pendwn_irq;
+ return error;
}
/* Mask PEN_DOWN and TSI_READY events */
@@ -296,25 +312,13 @@ static int da9052_ts_probe(struct platform_device *pdev)
error = da9052_configure_tsi(tsi);
if (error)
- goto err_free_datardy_irq;
+ return error;
error = input_register_device(tsi->dev);
if (error)
- goto err_free_datardy_irq;
-
- platform_set_drvdata(pdev, tsi);
+ return error;
return 0;
-
-err_free_datardy_irq:
- da9052_free_irq(tsi->da9052, DA9052_IRQ_TSIREADY, tsi);
-err_free_pendwn_irq:
- da9052_free_irq(tsi->da9052, DA9052_IRQ_PENDOWN, tsi);
-err_free_mem:
- kfree(tsi);
- input_free_device(input_dev);
-
- return error;
}
static int da9052_ts_remove(struct platform_device *pdev)
@@ -323,12 +327,6 @@ static int da9052_ts_remove(struct platform_device *pdev)
da9052_reg_write(tsi->da9052, DA9052_LDO9_REG, 0x19);
- da9052_free_irq(tsi->da9052, DA9052_IRQ_TSIREADY, tsi);
- da9052_free_irq(tsi->da9052, DA9052_IRQ_PENDOWN, tsi);
-
- input_unregister_device(tsi->dev);
- kfree(tsi);
-
return 0;
}
--
end-of-patch for input: use device managed memory in da9052 touchscreen driver V1
^ permalink raw reply related
* Re: [PATCH 2/2] HID: sony: Add LED controls for Dualshock 4
From: Antonio Ospite @ 2014-01-09 10:45 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input
In-Reply-To: <alpine.DEB.2.10.1401031542220.29517@franz-virtual-machine>
On Fri, 3 Jan 2014 16:03:46 -0500 (EST)
Frank Praznik <frank.praznik@oh.rr.com> wrote:
> This patch builds on the previous patch and adds controls for the light
Just use "Add controls for ..."
> bar on the Dualshock 4. The light bar contains a red, green and blue LED
> that can independently vary in brightness from 0 to 255. The LED system
> in the module had to be extended to support a full byte per led for
> storing the brightness level as well as storing a count of LEDs on each
> device. Like the Sixaxis, LED status is set in the worker function by
> setting certain bytes in the data packet.
>
> Like the force feedback patch, this meant to be applied against the
> for-3.14/sony branch in jiros/hid.git
>
Annotation :)
Ah and remember to CC Jiri Kosina when you send the v2, you can use
scripts/get_maintainer.pl to get some _hints_ about where to send a
patch.
Some more nitpicking below.
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
>
> ---
> drivers/hid/hid-sony.c | 77 ++++++++++++++++++++++++++++++++------------------
> 1 file changed, 49 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index f42c866..445b5f2 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -40,7 +40,9 @@
> #define PS3REMOTE BIT(4)
> #define DUALSHOCK4_CONTROLLER BIT(5)
>
> -#define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER)
> +#define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER | DUALSHOCK4_CONTROLLER)
> +
> +#define MAX_LEDS 4
>
> static const u8 sixaxis_rdesc_fixup[] = {
> 0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C,
> @@ -227,7 +229,7 @@ static const unsigned int buzz_keymap[] = {
>
> struct sony_sc {
> struct hid_device *hdev;
> - struct led_classdev *leds[4];
> + struct led_classdev *leds[MAX_LEDS];
> unsigned long quirks;
> struct work_struct state_worker;
>
> @@ -236,7 +238,8 @@ struct sony_sc {
> __u8 right;
> #endif
>
> - __u8 led_state;
> + __u8 led_state[MAX_LEDS];
> + __u8 led_count;
> };
>
> static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
> @@ -447,7 +450,7 @@ static int sixaxis_set_operational_bt(struct hid_device *hdev)
> return hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
> }
>
> -static void buzz_set_leds(struct hid_device *hdev, int leds)
> +static void buzz_set_leds(struct hid_device *hdev, const __u8 *leds)
> {
> struct list_head *report_list =
> &hdev->report_enum[HID_OUTPUT_REPORT].report_list;
> @@ -456,23 +459,27 @@ static void buzz_set_leds(struct hid_device *hdev, int leds)
> __s32 *value = report->field[0]->value;
>
> value[0] = 0x00;
> - value[1] = (leds & 1) ? 0xff : 0x00;
> - value[2] = (leds & 2) ? 0xff : 0x00;
> - value[3] = (leds & 4) ? 0xff : 0x00;
> - value[4] = (leds & 8) ? 0xff : 0x00;
> + value[1] = leds[0] ? 0xff : 0x00;
> + value[2] = leds[1] ? 0xff : 0x00;
> + value[3] = leds[2] ? 0xff : 0x00;
> + value[4] = leds[3] ? 0xff : 0x00;
> value[5] = 0x00;
> value[6] = 0x00;
> hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
> }
>
> -static void sony_set_leds(struct hid_device *hdev, __u8 leds)
> +static void sony_set_leds(struct hid_device *hdev, const __u8 *leds, int count)
> {
> struct sony_sc *drv_data = hid_get_drvdata(hdev);
> -
> - if (drv_data->quirks & BUZZ_CONTROLLER) {
> + int n;
> +
> + BUG_ON(count > MAX_LEDS);
> +
> + if (drv_data->quirks & BUZZ_CONTROLLER && count == 4) {
> buzz_set_leds(hdev, leds);
> - } else if (drv_data->quirks & SIXAXIS_CONTROLLER_USB) {
> - drv_data->led_state = leds;
> + } else if ((drv_data->quirks & SIXAXIS_CONTROLLER_USB) || (drv_data->quirks & DUALSHOCK4_CONTROLLER)) {
> + for(n = 0; n < count; n++)
> + drv_data->led_state[n] = leds[n];
> schedule_work(&drv_data->state_worker);
> }
> }
> @@ -492,15 +499,11 @@ static void sony_led_set_brightness(struct led_classdev *led,
> return;
> }
>
> - for (n = 0; n < 4; n++) {
> + for (n = 0; n < drv_data->led_count; n++) {
> if (led == drv_data->leds[n]) {
> - int on = !!(drv_data->led_state & (1 << n));
> - if (value == LED_OFF && on) {
> - drv_data->led_state &= ~(1 << n);
> - sony_set_leds(hdev, drv_data->led_state);
> - } else if (value != LED_OFF && !on) {
> - drv_data->led_state |= (1 << n);
> - sony_set_leds(hdev, drv_data->led_state);
> + if (value != drv_data->led_state[n]) {
> + drv_data->led_state[n] = value;
> + sony_set_leds(hdev, drv_data->led_state, drv_data->led_count);
> }
> break;
> }
> @@ -522,9 +525,9 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
> return LED_OFF;
> }
>
> - for (n = 0; n < 4; n++) {
> + for (n = 0; n < drv_data->led_count; n++) {
> if (led == drv_data->leds[n]) {
> - on = !!(drv_data->led_state & (1 << n));
> + on = !!(drv_data->led_state[n]);
> break;
> }
> }
> @@ -541,7 +544,7 @@ static void sony_leds_remove(struct hid_device *hdev)
> drv_data = hid_get_drvdata(hdev);
> BUG_ON(!(drv_data->quirks & SONY_LED_SUPPORT));
>
> - for (n = 0; n < 4; n++) {
> + for (n = 0; n < drv_data->led_count; n++) {
> led = drv_data->leds[n];
> drv_data->leds[n] = NULL;
> if (!led)
> @@ -549,17 +552,21 @@ static void sony_leds_remove(struct hid_device *hdev)
> led_classdev_unregister(led);
> kfree(led);
> }
> +
> + drv_data->led_count = 0;
> }
>
> static int sony_leds_init(struct hid_device *hdev)
> {
> struct sony_sc *drv_data;
> int n, ret = 0;
> + int max_brightness = 1;
you could avoid initialization here and set the value in the check
below, it'd look more symmetrical to me.
> struct led_classdev *led;
> size_t name_sz;
> char *name;
> size_t name_len;
> const char *name_fmt;
> + static const __u8 initial_values[MAX_LEDS] = { 0x00, 0x00, 0x00, 0x00 };
>
> drv_data = hid_get_drvdata(hdev);
> BUG_ON(!(drv_data->quirks & SONY_LED_SUPPORT));
> @@ -574,15 +581,22 @@ static int sony_leds_init(struct hid_device *hdev)
> name_len = strlen("::sony#");
> name_fmt = "%s::sony%d";
> }
> +
> + if (drv_data->quirks & DUALSHOCK4_CONTROLLER) {
> + drv_data->led_count = 3;
> + max_brightness = 255;
> + }
> + else
> + drv_data->led_count = 4;
>
Also the else block requires curly braces, see Documentation/CodingStyle
maybe scripts/checkpatch.pl catches these issues.
> /* Clear LEDs as we have no way of reading their initial state. This is
> * only relevant if the driver is loaded after somebody actively set the
> * LEDs to on */
> - sony_set_leds(hdev, 0x00);
> + sony_set_leds(hdev, initial_values, drv_data->led_count);
>
> name_sz = strlen(dev_name(&hdev->dev)) + name_len + 1;
>
> - for (n = 0; n < 4; n++) {
> + for (n = 0; n < drv_data->led_count; n++) {
> led = kzalloc(sizeof(struct led_classdev) + name_sz, GFP_KERNEL);
> if (!led) {
> hid_err(hdev, "Couldn't allocate memory for LED %d\n", n);
> @@ -594,7 +608,7 @@ static int sony_leds_init(struct hid_device *hdev)
> snprintf(name, name_sz, name_fmt, dev_name(&hdev->dev), n + 1);
> led->name = name;
> led->brightness = 0;
> - led->max_brightness = 1;
> + led->max_brightness = max_brightness;
> led->brightness_get = sony_led_get_brightness;
> led->brightness_set = sony_led_set_brightness;
>
> @@ -635,7 +649,10 @@ static void sixaxis_state_worker(struct work_struct *work)
> buf[5] = sc->left;
> #endif
>
> - buf[10] |= (sc->led_state & 0xf) << 1;
> + buf[10] |= sc->led_state[0] << 1;
> + buf[10] |= sc->led_state[1] << 2;
> + buf[10] |= sc->led_state[2] << 3;
> + buf[10] |= sc->led_state[3] << 4;
>
> sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
> HID_OUTPUT_REPORT);
> @@ -660,6 +677,10 @@ static void dualshock4_state_worker(struct work_struct *work)
> buf[5] = sc->left;
> #endif
>
> + buf[6] = sc->led_state[0];
> + buf[7] = sc->led_state[1];
> + buf[8] = sc->led_state[2];
> +
> sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
> HID_OUTPUT_REPORT);
> }
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
^ permalink raw reply
* Re: [PATCH 1/2] HID: sony: Add force feedback for the Dualshock 4
From: Antonio Ospite @ 2014-01-09 10:38 UTC (permalink / raw)
To: Frank Praznik; +Cc: linux-input
In-Reply-To: <alpine.DEB.2.10.1401031518010.29517@franz-virtual-machine>
On Fri, 3 Jan 2014 16:03:10 -0500 (EST)
Frank Praznik <frank.praznik@oh.rr.com> wrote:
Hi Frank,
just some notes about the style.
> This patch adds force feedback support for Sony's Dualshock 4 controller.
You can just use the imperative form in the long commit message too:
"Add force feedback support for Sony's Dualshock 4 controller..."
No need to repeat "This patch ..." in the commit message.
> It does this by adding a device ID for the new controller and creating a
> new dualshock4_worker function to send data commands formatted for the
> controller. Unlike the Sixaxis, the Dualshock 4 requires a magnitude
> value for the small motor so the actual value is now sent to the worker
> function and the sixaxis worker was modified to clamp it to 1 or 0 as required.
>
> This patch is built against the for-3.14/sony branch in jikos/hid.git and
> is compatible with the recent fixes.
this last sentence is more like an annotation, put annotations after the
three dashes below, just before the diffstat, don't put them in the
commit message because they will become outdated and they will look odd
when someone reads the project history at some point in the future.
An alternative is to generate a cover-letter with git-format-patch and
mention there the prerequisites for the patch series.
> Signed-off-by Frank Praznik <frank.praznik@oh.rr.com>
>
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/hid-sony.c | 41 +++++++++++++++++++++++++++++++++++++----
> 2 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 60336f06..ebbd292 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -769,6 +769,7 @@
> #define USB_DEVICE_ID_SONY_NAVIGATION_CONTROLLER 0x042f
> #define USB_DEVICE_ID_SONY_BUZZ_CONTROLLER 0x0002
> #define USB_DEVICE_ID_SONY_WIRELESS_BUZZ_CONTROLLER 0x1000
> +#define USB_DEVICE_ID_SONY_PS4_CONTROLLER 0x05c4
>
You can align the value with the other ones using two TABs in this case.
> #define USB_VENDOR_ID_SOUNDGRAPH 0x15c2
> #define USB_DEVICE_ID_SOUNDGRAPH_IMON_FIRST 0x0034
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index f57ab5e..f42c866 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -38,6 +38,7 @@
> #define SIXAXIS_CONTROLLER_BT BIT(2)
> #define BUZZ_CONTROLLER BIT(3)
> #define PS3REMOTE BIT(4)
> +#define DUALSHOCK4_CONTROLLER BIT(5\)
most of the entries are aligned with spaces here so use spaces,
PS3REMOTE will be fixed eventually. Also, what about the backslash
before the parenthesis?
> #define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER)
>
> @@ -615,7 +616,7 @@ error_leds:
> return ret;
> }
>
> -static void sony_state_worker(struct work_struct *work)
> +static void sixaxis_state_worker(struct work_struct *work)
this is a rename, I understand it is in preparation for
dualshock4_state_worker() but it's not _directly_ related to the topic
of the patch; split cleanups and cosmetic changes from functional
changes, it is easier to review and validate logically contained
patches.
> {
> struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
> unsigned char buf[] = {
> @@ -630,7 +631,7 @@ static void sony_state_worker(struct work_struct *work)
> };
>
> #ifdef CONFIG_SONY_FF
> - buf[3] = sc->right;
> + buf[3] = sc->right ? 1 : 0;
> buf[5] = sc->left;
> #endif
>
> @@ -640,6 +641,29 @@ static void sony_state_worker(struct work_struct *work)
> HID_OUTPUT_REPORT);
> }
>
> +static void dualshock4_state_worker(struct work_struct *work)
> +{
> + struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
> + unsigned char buf[] = {
> + 0x05,
> + 0x03, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00, 0x00, 0x00, 0x00, 0x00,
> + 0x00,
> + };
> +
> +#ifdef CONFIG_SONY_FF
> + buf[4] = sc->right;
> + buf[5] = sc->left;
> +#endif
> +
> + sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
> + HID_OUTPUT_REPORT);
> +}
> +
> #ifdef CONFIG_SONY_FF
> static int sony_play_effect(struct input_dev *dev, void *data,
> struct ff_effect *effect)
> @@ -651,7 +675,7 @@ static int sony_play_effect(struct input_dev *dev, void *data,
> return 0;
>
> sc->left = effect->u.rumble.strong_magnitude / 256;
> - sc->right = effect->u.rumble.weak_magnitude ? 1 : 0;
> + sc->right = effect->u.rumble.weak_magnitude / 256;
>
> schedule_work(&sc->state_worker);
> return 0;
> @@ -724,10 +748,14 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
> if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
> hdev->hid_output_raw_report = sixaxis_usb_output_raw_report;
> ret = sixaxis_set_operational_usb(hdev);
> - INIT_WORK(&sc->state_worker, sony_state_worker);
> + INIT_WORK(&sc->state_worker, sixaxis_state_worker);
> }
> else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
> ret = sixaxis_set_operational_bt(hdev);
> + else if (sc->quirks & DUALSHOCK4_CONTROLLER) {
> + ret = 0;
> + INIT_WORK(&sc->state_worker, dualshock4_state_worker);
> + }
> else
> ret = 0;
>
> @@ -787,6 +815,11 @@ static const struct hid_device_id sony_devices[] = {
> /* Logitech Harmony Adapter for PS3 */
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3),
> .driver_data = PS3REMOTE },
> + /* Sony Dualshock 4 controllers for PS4 */
> + { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER),
> + .driver_data = DUALSHOCK4_CONTROLLER },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS4_CONTROLLER),
> + .driver_data = DUALSHOCK4_CONTROLLER },
> { }
> };
> MODULE_DEVICE_TABLE(hid, sony_devices);
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Antonio Ospite
http://ao2.it
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
^ permalink raw reply
* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Dmitry Torokhov @ 2014-01-09 8:28 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <20140109080454.GA27160@core.coreip.homeip.net>
On Thu, Jan 09, 2014 at 12:04:54AM -0800, Dmitry Torokhov wrote:
> On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
> > This is a trivial change to replace the sprintf loop with snprintf using
> > up-to-date format capability.
>
> Hmm, how about we do this instead:
And another small one...
Input: synaptics-rmi4 - transport name should be a const pointer
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/rmi4/rmi_bus.h | 2 +-
drivers/input/rmi4/rmi_i2c.c | 4 +---
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/input/rmi4/rmi_bus.h b/drivers/input/rmi4/rmi_bus.h
index 65dd934..3e8b57a 100644
--- a/drivers/input/rmi4/rmi_bus.h
+++ b/drivers/input/rmi4/rmi_bus.h
@@ -148,7 +148,7 @@ struct rmi_driver {
* @att_count Number of times ATTN assertions have been handled.
*/
struct rmi_transport_info {
- char *proto;
+ const char *proto;
long tx_count;
long tx_bytes;
long tx_errs;
diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index ea01823..ebe74ec 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -38,8 +38,6 @@ struct rmi_i2c_data {
#define RMI_PAGE_SELECT_REGISTER 0xff
#define RMI_I2C_PAGE(addr) (((addr) >> 8) & 0xff)
-static char *xport_proto_name = "i2c";
-
/*
* rmi_set_page - Set RMI page
* @xport: The pointer to the rmi_transport_dev struct
@@ -217,7 +215,7 @@ static int rmi_i2c_probe(struct i2c_client *client,
xport->write_block = rmi_i2c_write_block;
xport->read_block = rmi_i2c_read_block;
- xport->info.proto = xport_proto_name;
+ xport->info.proto = "i2c";
mutex_init(&data->page_mutex);
--
Dmitry
^ permalink raw reply related
* Re: [PATCH] input synaptics-rmi4: move rmi_f01 query register parsing to its own function
From: Dmitry Torokhov @ 2014-01-09 8:18 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <1389142930-26279-1-git-send-email-cheiny@synaptics.com>
On Tue, Jan 07, 2014 at 05:02:10PM -0800, Christopher Heiny wrote:
> In the near future, query register parsing is going to get a lot more
> complicated. It's also going to be needed by the reflash code. Now is the
> time to move this from the F01 initialization into its own function,
> while the code is still fairly simple.
>
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
Applied, thank you.
> ---
>
> drivers/input/rmi4/rmi_f01.c | 71 +++++++++++++++++++++++++++-----------------
> 1 file changed, 43 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index d547633..1cb11ea 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -167,6 +167,46 @@ static int rmi_f01_alloc_memory(struct rmi_function *fn,
> return 0;
> }
>
> +static int rmi_f01_read_properties(struct rmi_device *rmi_dev,
> + u16 query_base_addr,
> + struct f01_basic_properties *props)
> +{
> + u8 basic_query[RMI_F01_BASIC_QUERY_LEN];
> + int error;
> +
> + error = rmi_read_block(rmi_dev, query_base_addr,
> + basic_query, sizeof(basic_query));
> + if (error < 0) {
> + dev_err(&rmi_dev->dev, "Failed to read device query registers.\n");
> + return error;
> + }
> +
> + /* Now parse what we got */
> + props->manufacturer_id = basic_query[0];
> +
> + props->has_lts = basic_query[1] & RMI_F01_QRY1_HAS_LTS;
> + props->has_adjustable_doze =
> + basic_query[1] & RMI_F01_QRY1_HAS_ADJ_DOZE;
> + props->has_adjustable_doze_holdoff =
> + basic_query[1] & RMI_F01_QRY1_HAS_ADJ_DOZE_HOFF;
> +
> + snprintf(props->dom, sizeof(props->dom),
> + "20%02x%02x%02x",
> + basic_query[5] & RMI_F01_QRY5_YEAR_MASK,
> + basic_query[6] & RMI_F01_QRY6_MONTH_MASK,
> + basic_query[7] & RMI_F01_QRY7_DAY_MASK);
> +
> + memcpy(props->product_id, &basic_query[11],
> + RMI_PRODUCT_ID_LENGTH);
> + props->product_id[RMI_PRODUCT_ID_LENGTH] = '\0';
> +
> + props->productinfo =
> + ((basic_query[2] & RMI_F01_QRY2_PRODINFO_MASK) << 7) |
> + (basic_query[3] & RMI_F01_QRY2_PRODINFO_MASK);
> +
> + return 0;
> +}
> +
> static int rmi_f01_initialize(struct rmi_function *fn)
> {
> u8 temp;
> @@ -176,7 +216,6 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> struct rmi_driver_data *driver_data = dev_get_drvdata(&rmi_dev->dev);
> struct f01_data *data = fn->data;
> struct rmi_device_platform_data *pdata = to_rmi_platform_data(rmi_dev);
> - u8 basic_query[RMI_F01_BASIC_QUERY_LEN];
>
> mutex_init(&data->control_mutex);
>
> @@ -248,36 +287,12 @@ static int rmi_f01_initialize(struct rmi_function *fn)
> return error;
> }
>
> - error = rmi_read_block(rmi_dev, fn->fd.query_base_addr,
> - basic_query, sizeof(basic_query));
> + error = rmi_f01_read_properties(rmi_dev, fn->fd.query_base_addr,
> + &data->properties);
> if (error < 0) {
> - dev_err(&fn->dev, "Failed to read device query registers.\n");
> + dev_err(&fn->dev, "Failed to read F01 properties.\n");
> return error;
> }
> -
> - /* Now parse what we got */
> - data->properties.manufacturer_id = basic_query[0];
> -
> - data->properties.has_lts = basic_query[1] & RMI_F01_QRY1_HAS_LTS;
> - data->properties.has_adjustable_doze =
> - basic_query[1] & RMI_F01_QRY1_HAS_ADJ_DOZE;
> - data->properties.has_adjustable_doze_holdoff =
> - basic_query[1] & RMI_F01_QRY1_HAS_ADJ_DOZE_HOFF;
> -
> - snprintf(data->properties.dom, sizeof(data->properties.dom),
> - "20%02x%02x%02x",
> - basic_query[5] & RMI_F01_QRY5_YEAR_MASK,
> - basic_query[6] & RMI_F01_QRY6_MONTH_MASK,
> - basic_query[7] & RMI_F01_QRY7_DAY_MASK);
> -
> - memcpy(data->properties.product_id, &basic_query[11],
> - RMI_PRODUCT_ID_LENGTH);
> - data->properties.product_id[RMI_PRODUCT_ID_LENGTH] = '\0';
> -
> - data->properties.productinfo =
> - ((basic_query[2] & RMI_F01_QRY2_PRODINFO_MASK) << 7) |
> - (basic_query[3] & RMI_F01_QRY2_PRODINFO_MASK);
> -
> dev_info(&fn->dev, "found RMI device, manufacturer: %s, product: %s\n",
> data->properties.manufacturer_id == 1 ?
> "Synaptics" : "unknown",
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input synaptics-rmi4 trivial: rmi_f01.c tidy-up
From: Dmitry Torokhov @ 2014-01-09 8:11 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <1389139662-6590-1-git-send-email-cheiny@synaptics.com>
On Tue, Jan 07, 2014 at 04:07:42PM -0800, Christopher Heiny wrote:
> This has two trivial changes:
>
> 1) use CONFIG_PM_SLEEP instead of CONFIG_PM for consistency.
>
> 2) Update the copyright date.
>
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Applied, thank you.
>
> ---
> drivers/input/rmi4/rmi_f01.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
> index 628b082..d547633 100644
> --- a/drivers/input/rmi4/rmi_f01.c
> +++ b/drivers/input/rmi4/rmi_f01.c
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2011-2012 Synaptics Incorporated
> + * Copyright (c) 2011-2014 Synaptics Incorporated
> * Copyright (c) 2011 Unixphere
> *
> * This program is free software; you can redistribute it and/or modify it
> @@ -138,7 +138,7 @@ struct f01_data {
> int irq_count;
> int num_of_irq_regs;
>
> -#ifdef CONFIG_PM
> +#ifdef CONFIG_PM_SLEEP
> bool suspended;
> bool old_nosleep;
> #endif
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: synaptics-rmi4 - cleanup rmi_i2c_probe()
From: Dmitry Torokhov @ 2014-01-09 8:06 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <1389219092-32042-1-git-send-email-cheiny@synaptics.com>
On Wed, Jan 08, 2014 at 02:11:32PM -0800, Christopher Heiny wrote:
> Moves i2c_check_functionality to occur before the gpio_config() call. This
> can catch some issues that would otherwise result in mysterious problems
> in gpio_config().
>
> Reduces debugging output; updates remaining output to be more accurate.
>
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Applied, thank you.
>
> ---
>
> drivers/input/rmi4/rmi_i2c.c | 18 ++++++++----------
> 1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
> index 43b0e53..b4bd8ae 100644
> --- a/drivers/input/rmi4/rmi_i2c.c
> +++ b/drivers/input/rmi4/rmi_i2c.c
> @@ -274,26 +274,24 @@ static int rmi_i2c_probe(struct i2c_client *client,
> dev_err(&client->dev, "no platform data\n");
> return -EINVAL;
> }
> - dev_info(&client->dev, "Probing %s at %#02x (IRQ %d).\n",
> + dev_dbg(&client->dev, "Probing %s at %#02x (GPIO %d).\n",
> pdata->sensor_name ? pdata->sensor_name : "-no name-",
> client->addr, pdata->attn_gpio);
>
> + retval = i2c_check_functionality(client->adapter, I2C_FUNC_I2C);
> + if (!retval) {
> + dev_err(&client->dev, "i2c_check_functionality error %d.\n",
> + retval);
> + return retval;
> + }
> +
> if (pdata->gpio_config) {
> - dev_dbg(&client->dev, "Configuring GPIOs.\n");
> retval = pdata->gpio_config(pdata->gpio_data, true);
> if (retval < 0) {
> dev_err(&client->dev, "Failed to configure GPIOs, code: %d.\n",
> retval);
> return retval;
> }
> - dev_info(&client->dev, "Done with GPIO configuration.\n");
> - }
> -
> - retval = i2c_check_functionality(client->adapter, I2C_FUNC_I2C);
> - if (!retval) {
> - dev_err(&client->dev, "i2c_check_functionality error %d.\n",
> - retval);
> - return retval;
> }
>
> xport = devm_kzalloc(&client->dev, sizeof(struct rmi_transport_dev),
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] input: synaptics-rmi4 - use snprintf instead of sprintf in rmi_i2c.c
From: Dmitry Torokhov @ 2014-01-09 8:04 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <1389230319-4737-1-git-send-email-cheiny@synaptics.com>
On Wed, Jan 08, 2014 at 05:18:39PM -0800, Christopher Heiny wrote:
> This is a trivial change to replace the sprintf loop with snprintf using
> up-to-date format capability.
Hmm, how about we do this instead:
Input: synaptics-rmi4 - clean up debug handling in rmi_i2c
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Kernel now has standard facility to format and print binary buffers, let's
use it. By doing so we no longer need to allocate memory for debug buffers
and we can let debugfs code go as well.
We have a new limitation however - output is limited to the first 64 bytes
of the buffer. However if we really need to capture larger messages dumping
them in dmesg is not right interface anyway.
Also clean up extra includes, while we are at it.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/rmi4/rmi_i2c.c | 131 +++++-------------------------------------
1 file changed, 15 insertions(+), 116 deletions(-)
diff --git a/drivers/input/rmi4/rmi_i2c.c b/drivers/input/rmi4/rmi_i2c.c
index b33074c..c2ad1aa 100644
--- a/drivers/input/rmi4/rmi_i2c.c
+++ b/drivers/input/rmi4/rmi_i2c.c
@@ -8,16 +8,10 @@
*/
#include <linux/kernel.h>
-#include <linux/delay.h>
#include <linux/i2c.h>
-#include <linux/interrupt.h>
-#include <linux/kconfig.h>
-#include <linux/lockdep.h>
#include <linux/module.h>
-#include <linux/pm.h>
#include <linux/rmi.h>
#include <linux/slab.h>
-#include <linux/uaccess.h>
#include "rmi_driver.h"
#define BUFFER_SIZE_INCREMENT 32
@@ -31,11 +25,6 @@
*
* @tx_buf: Buffer used for transmitting data to the sensor over i2c.
* @tx_buf_size: Size of the buffer
- * @debug_buf: Buffer used for exposing buffer contents using dev_dbg
- * @debug_buf_size: Size of the debug buffer.
- *
- * @comms_debug: Latest data read/written for debugging I2C communications
- * @debugfs_comms: Debugfs file for debugging I2C communications
*/
struct rmi_i2c_data {
struct mutex page_mutex;
@@ -44,56 +33,8 @@ struct rmi_i2c_data {
u8 *tx_buf;
int tx_buf_size;
- u8 *debug_buf;
- int debug_buf_size;
-
- u32 comms_debug;
-#ifdef CONFIG_RMI4_DEBUG
- struct dentry *debugfs_comms;
-#endif
};
-#ifdef CONFIG_RMI4_DEBUG
-
-static int setup_debugfs(struct rmi_device *rmi_dev, struct rmi_i2c_data *data)
-{
- if (!rmi_dev->debugfs_root)
- return -ENODEV;
-
- data->debugfs_comms = debugfs_create_bool("comms_debug",
- (S_IRUGO | S_IWUGO), rmi_dev->debugfs_root,
- &data->comms_debug);
- if (!data->debugfs_comms || IS_ERR(data->debugfs_comms)) {
- dev_warn(&rmi_dev->dev,
- "Failed to create debugfs comms_debug.\n");
- data->debugfs_comms = NULL;
- }
-
- return 0;
-}
-
-static void teardown_debugfs(struct rmi_i2c_data *data)
-{
- if (data->debugfs_comms)
- debugfs_remove(data->debugfs_comms);
-}
-
-#else
-
-static inline int setup_debugfs(struct rmi_device *rmi_dev,
- struct rmi_i2c_data *data)
-{
- return 0;
-}
-
-static inline void teardown_debugfs(struct rmi_i2c_data *data)
-{
-}
-
-#endif
-
-#define COMMS_DEBUG(data) (IS_ENABLED(CONFIG_RMI4_DEBUG) && data->comms_debug)
-
#define RMI_PAGE_SELECT_REGISTER 0xff
#define RMI_I2C_PAGE(addr) (((addr) >> 8) & 0xff)
@@ -120,8 +61,7 @@ static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
u8 txbuf[2] = {RMI_PAGE_SELECT_REGISTER, page};
int retval;
- if (COMMS_DEBUG(data))
- dev_dbg(&client->dev, "writes 3 bytes: %02x %02x\n",
+ dev_dbg(&client->dev, "writes 3 bytes: %02x %02x\n",
txbuf[0], txbuf[1]);
xport->info.tx_count++;
xport->info.tx_bytes += sizeof(txbuf);
@@ -136,34 +76,6 @@ static int rmi_set_page(struct rmi_transport_dev *xport, u8 page)
return 0;
}
-static int copy_to_debug_buf(struct device *dev, struct rmi_i2c_data *data,
- const u8 *buf, const int len) {
- int i;
- int n = 0;
- char *temp;
- int dbg_size = 3 * len + 1;
-
- if (!data->debug_buf || data->debug_buf_size < dbg_size) {
- if (data->debug_buf)
- devm_kfree(dev, data->debug_buf);
- data->debug_buf_size = dbg_size + BUFFER_SIZE_INCREMENT;
- data->debug_buf = devm_kzalloc(dev, data->debug_buf_size,
- GFP_KERNEL);
- if (!data->debug_buf) {
- data->debug_buf_size = 0;
- return -ENOMEM;
- }
- }
- temp = data->debug_buf;
-
- for (i = 0; i < len; i++) {
- n = sprintf(temp, " %02x", buf[i]);
- temp += n;
- }
-
- return 0;
-}
-
static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
const void *buf, const int len)
{
@@ -195,12 +107,8 @@ static int rmi_i2c_write_block(struct rmi_transport_dev *xport, u16 addr,
goto exit;
}
- if (COMMS_DEBUG(data)) {
- int rc = copy_to_debug_buf(&client->dev, data, (u8 *) buf, len);
- if (!rc)
- dev_dbg(&client->dev, "writes %d bytes at %#06x:%s\n",
- len, addr, data->debug_buf);
- }
+ dev_dbg(&client->dev,
+ "writes %d bytes at %#06x: %*ph\n", len, addr, len, buf);
xport->info.tx_count++;
xport->info.tx_bytes += tx_size;
@@ -232,8 +140,7 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
goto exit;
}
- if (COMMS_DEBUG(data))
- dev_dbg(&client->dev, "writes 1 bytes: %02x\n", txbuf[0]);
+ dev_dbg(&client->dev, "writes 1 bytes: %02x\n", txbuf[0]);
xport->info.tx_count++;
xport->info.tx_bytes += sizeof(txbuf);
@@ -244,18 +151,16 @@ static int rmi_i2c_read_block(struct rmi_transport_dev *xport, u16 addr,
goto exit;
}
- retval = i2c_master_recv(client, (u8 *) buf, len);
+ retval = i2c_master_recv(client, buf, len);
xport->info.rx_count++;
xport->info.rx_bytes += len;
if (retval < 0)
xport->info.rx_errs++;
- else if (COMMS_DEBUG(data)) {
- int rc = copy_to_debug_buf(&client->dev, data, (u8 *) buf, len);
- if (!rc)
- dev_dbg(&client->dev, "read %d bytes at %#06x:%s\n",
- len, addr, data->debug_buf);
- }
+ else
+ dev_dbg(&client->dev,
+ "read %d bytes at %#06x: %*ph\n",
+ len, addr, len, buf);
exit:
mutex_unlock(&data->page_mutex);
@@ -265,9 +170,9 @@ exit:
static int rmi_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
+ const struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
struct rmi_transport_dev *xport;
struct rmi_i2c_data *data;
- struct rmi_device_platform_data *pdata = client->dev.platform_data;
int retval;
if (!pdata) {
@@ -318,7 +223,8 @@ static int rmi_i2c_probe(struct i2c_client *client,
mutex_init(&data->page_mutex);
- /* Setting the page to zero will (a) make sure the PSR is in a
+ /*
+ * Setting the page to zero will (a) make sure the PSR is in a
* known state, and (b) make sure we can talk to the device.
*/
retval = rmi_set_page(xport, 0);
@@ -335,11 +241,6 @@ static int rmi_i2c_probe(struct i2c_client *client,
}
i2c_set_clientdata(client, xport);
- retval = setup_debugfs(xport->rmi_dev, data);
- if (retval < 0)
- dev_warn(&client->dev, "Failed to setup debugfs. Code: %d.\n",
- retval);
-
dev_info(&client->dev, "registered rmi i2c driver at %#04x.\n",
client->addr);
return 0;
@@ -353,14 +254,12 @@ err_gpio:
static int rmi_i2c_remove(struct i2c_client *client)
{
struct rmi_transport_dev *xport = i2c_get_clientdata(client);
- struct rmi_device_platform_data *pd = client->dev.platform_data;
-
- teardown_debugfs(xport->data);
+ struct rmi_device_platform_data *pdata = dev_get_platdata(&client->dev);
rmi_unregister_transport_device(xport);
- if (pd->gpio_config)
- pd->gpio_config(&pd->gpio_data, false);
+ if (pdata->gpio_config)
+ pdata->gpio_config(&pdata->gpio_data, false);
return 0;
}
--
Dmitry
^ permalink raw reply related
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