* Re: [PATCH v4 2/4] Input: misc: Add haptic driver on max77693
From: Jaewon Kim @ 2014-09-12 1:38 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Samuel Ortiz, Lee Jones, Chanwoo Choi, linux-kernel, linux-input
In-Reply-To: <20140911171002.GC13083@core.coreip.homeip.net>
Hello Dmity Torokhov.
2014년 09월 12일 02:10에 Dmitry Torokhov 이(가) 쓴 글:
> On Thu, Sep 11, 2014 at 09:54:20PM +0900, Jaewon Kim wrote:
>> This patch add max77693-haptic device driver to support the haptic controller
>> on MAX77693. The MAX77693 is a Multifunction device with PMIC, CHARGER, LED,
>> MUIC, HAPTIC and the patch is haptic device driver in the MAX77693. This driver
>> support external pwm and LRA(Linear Resonant Actuator) motor. User can control
>> the haptic driver by using force feedback framework.
>>
>> Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
>> Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> How do we want to merge this?
>
thanks for review.
Please merge this input device patch only.
Another patchs will be merged by lee jones.
thanks
Jaewon Kim
--
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 3/3] psmouse: Add support for detecting FocalTech PS/2 touchpads
From: Dmitry Torokhov @ 2014-09-12 6:14 UTC (permalink / raw)
To: Hans de Goede; +Cc: Peter Hutterer, Benjamin Tissoires, linux-input
In-Reply-To: <5411DC61.2050003@redhat.com>
On Thu, Sep 11, 2014 at 07:31:13PM +0200, Hans de Goede wrote:
> Hi,
>
> On 09/11/2014 07:26 PM, Dmitry Torokhov wrote:
> > Hi Hans,
> >
> > On Thu, Sep 11, 2014 at 10:50:47AM +0200, Hans de Goede wrote:
> >> @@ -722,6 +723,13 @@ static int psmouse_extensions(struct psmouse *psmouse,
> >> {
> >> bool synaptics_hardware = false;
> >>
> >> +/* Always check for focaltech, this is safe as it uses pnp-id matching */
> >> + if (psmouse_do_detect(focaltech_detect, psmouse, set_properties) == 0) {
> >> + /* Not supported yet, use bare protocol */
> >> + psmouse_max_proto = max_proto = PSMOUSE_PS2;
> >> + goto reset_to_defaults;
> >
> > Why do we need to jump to a new label instead of simply saying
>
> Once we had figured out that psmouse.proto=bare at least made this
> touchpad work in mouse emulation mode, that is the first thing I did, but it
> is not enough. We also need to set psmouse_max_proto to short-circuit
> psmouse_initialize and do the reset.
I'd rather we did the reset in focal_init() then. We'll need it anyway
later.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v4 2/4] Input: misc: Add haptic driver on max77693
From: Dmitry Torokhov @ 2014-09-12 6:43 UTC (permalink / raw)
To: Jaewon Kim
Cc: Samuel Ortiz, Lee Jones, Chanwoo Choi, linux-kernel, linux-input
In-Reply-To: <54124EA1.8060507@samsung.com>
On Fri, Sep 12, 2014 at 10:38:41AM +0900, Jaewon Kim wrote:
> Hello Dmity Torokhov.
>
> 2014년 09월 12일 02:10에 Dmitry Torokhov 이(가) 쓴 글:
> >On Thu, Sep 11, 2014 at 09:54:20PM +0900, Jaewon Kim wrote:
> >>This patch add max77693-haptic device driver to support the haptic controller
> >>on MAX77693. The MAX77693 is a Multifunction device with PMIC, CHARGER, LED,
> >>MUIC, HAPTIC and the patch is haptic device driver in the MAX77693. This driver
> >>support external pwm and LRA(Linear Resonant Actuator) motor. User can control
> >>the haptic driver by using force feedback framework.
> >>
> >>Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
> >>Acked-by: Chanwoo Choi <cw00.choi@samsung.com>
> >Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >
> >How do we want to merge this?
> >
> thanks for review.
>
> Please merge this input device patch only.
> Another patchs will be merged by lee jones.
Queued fro the next merge window (with some minor edits).
--
Dmitry
--
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 2/2] Input: misc: introduce palmas-pwrbutton
From: Dmitry Torokhov @ 2014-09-12 6:44 UTC (permalink / raw)
To: Nishanth Menon
Cc: Murphy, Dan, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20140911120119.GA17602@kahuna>
On Thu, Sep 11, 2014 at 07:01:19AM -0500, Nishanth Menon wrote:
> Hi Dimtry,
>
> On 14:13-20140910, Dmitry Torokhov wrote:
> > On Thu, Aug 21, 2014 at 02:01:43PM -0500, Nishanth Menon wrote:
> > > On 08/21/2014 01:03 PM, Dmitry Torokhov wrote:
> > >
> > > I believe I have taken care of other concerns on v2, but..Arrgh.. I
> > > did not reply to this comment..
> > > > BTW, I do not think you need to use of_node_get/put here, it's not going anywhere.
> > > It has been mentioned as a good practice to ensure we use get_put in
> > > to ensure reference count is appropriately maintained. So, I have'nt
> > > changed that in v3.
> >
> > You only need to maintain reference count if you pass the handle on.
> > Otherwise you'd have to do get/put every time you dereference something.
> >
> > Anyway, I did a few changes to the driver (no need to store current
> > state, do not fre einput device after unregister, etc.), could you
> > please tell me if the version below still works for you?
> [...]
> Thanks for taking the time to do all the changes - they are awesome and
> the resultant driver does work.
Thank you for [re]testing. I queued the driver for the next merge
window.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] input: Add ROHM BU21023/24 Dual touch support resistive touchscreens
From: Dmitry Torokhov @ 2014-09-12 7:23 UTC (permalink / raw)
To: Yoichi Yuasa; +Cc: linux-input
In-Reply-To: <20140804205142.e8d23082fc1b732fbb396ea2@linux-mips.org>
Hi Yoichi,
Just finishing a review I started a while back. I see that you posted v3
in the meantime, but I think majority of the comments are still
applicable.
On Mon, Aug 04, 2014 at 08:51:42PM +0900, Yoichi Yuasa wrote:
> Signed-off-by: Yoichi Yuasa <yuasa@linux-mips.org>
> ---
> drivers/input/touchscreen/Kconfig | 11 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/rohm_bu21023.c | 929 ++++++++++++++++++++++++++++++
> drivers/input/touchscreen/rohm_bu21023.h | 258 +++++++++
> 4 files changed, 1199 insertions(+)
> create mode 100644 drivers/input/touchscreen/rohm_bu21023.c
> create mode 100644 drivers/input/touchscreen/rohm_bu21023.h
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 6bb9a7d..9ae5c88 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -950,4 +950,15 @@ config TOUCHSCREEN_ZFORCE
> To compile this driver as a module, choose M here: the
> module will be called zforce_ts.
>
> +config TOUCHSCREEN_ROHM_BU21023
> + tristate "ROHM BU21023/24 Dual touch support resistive touchscreens"
> + depends on I2C
> + help
> + Say Y here if you have a touchscreen using ROHM BU21023/24.
> +
> + If unsure, say N.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called bu21023_ts.
> +
> endif
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 4be94fc..af766d0 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -52,6 +52,7 @@ obj-$(CONFIG_TOUCHSCREEN_USB_COMPOSITE) += usbtouchscreen.o
> obj-$(CONFIG_TOUCHSCREEN_PCAP) += pcap_ts.o
> obj-$(CONFIG_TOUCHSCREEN_PENMOUNT) += penmount.o
> obj-$(CONFIG_TOUCHSCREEN_PIXCIR) += pixcir_i2c_ts.o
> +obj-$(CONFIG_TOUCHSCREEN_ROHM_BU21023) += rohm_bu21023.o
> obj-$(CONFIG_TOUCHSCREEN_S3C2410) += s3c2410_ts.o
> obj-$(CONFIG_TOUCHSCREEN_ST1232) += st1232.o
> obj-$(CONFIG_TOUCHSCREEN_STMPE) += stmpe-ts.o
> diff --git a/drivers/input/touchscreen/rohm_bu21023.c b/drivers/input/touchscreen/rohm_bu21023.c
> new file mode 100644
> index 0000000..660b839
> --- /dev/null
> +++ b/drivers/input/touchscreen/rohm_bu21023.c
> @@ -0,0 +1,929 @@
> +/*
> + * ROHM BU21023/24 Dual touch support resistive touch screen driver
> + * Copyright (C) 2012 ROHM CO.,LTD.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * 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/delay.h>
> +#include <linux/firmware.h>
> +#include <linux/hrtimer.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/input/mt.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +#include "rohm_bu21023.h"
> +
> +struct rohm_ts_data {
> + struct i2c_client *client;
> + struct input_dev *input_dev;
> +
> + bool initialized;
> +
> + unsigned int untouch_count;
> + unsigned int single_touch_count;
> + unsigned int dual_touch_count;
> + unsigned int prev_touch_report;
> +};
> +
> +/*
> + * rohm_i2c_burst_read - execute combined I2C message for ROHM BU21023/24
> + * @adap: Handle to I2C bus
> + * @msgs: combined messages to execute
> + * @num: Number of messages to be executed.
> + *
> + * Returns negative errno, else the number of messages executed.
> + *
> + * Note
> + * In BU21023/24 burst read, stop condition is needed after "address write".
> + * Therefore, transmission is performed in 2 steps.
> + */
> +static inline int rohm_i2c_burst_read(struct i2c_adapter *adap,
> + struct i2c_msg *msgs, int num)
No need to declare functions in .c files inline, let compiler figure out what
worth inlining and what isn't.
> +{
> + int ret, i;
> +
> + if (!adap->algo->master_xfer) {
> + dev_err(&adap->dev, "I2C level transfers not supported\n");
> + return -EOPNOTSUPP;
> + }
This we better check in probe().
> +
> + i2c_lock_adapter(adap);
> +
> + for (i = 0; i < num; i++) {
> + ret = __i2c_transfer(adap, &msgs[i], 1);
> + if (ret < 0)
> + break;
> +
> + ret = i;
> + }
> +
> + i2c_unlock_adapter(adap);
> +
> + return ret;
> +}
Overall, I think this fucntion should just take the buffer and the size
to transfer and set up the 2 i2c message structure internally instead of
having callers supply them for you. So:
static int rohm_i2c_burst_read(struct i2c_client *client,
void *buf, size_t len)
{
...
}
You are also not handling partial transfers so make sure it return 0 on
success and threat partian trasfers as -EIO.
> +
> +static int rohm_ts_manual_calibration(struct rohm_ts_data *ts)
> +{
> + struct i2c_client *client = ts->client;
> + struct device *dev = &client->dev;
> + struct i2c_msg msg[2];
> + u8 buf[33];
> + u8 addr_buf; /* burst read start address */
> +
> + int retry;
> + bool success = false;
> + bool first_time = true;
> + bool calibration_done;
> +
> + u8 reg1, reg2, reg3;
> + s32 reg1_orig, reg2_orig, reg3_orig;
> + s32 val;
> +
> + int calib_x = 0, calib_y = 0;
> + int reg_x, reg_y;
> + int err_x, err_y;
> +
> + int err = 0, ret;
> + int i;
> +
> + addr_buf = PRM1_X_H;
> + msg[0].addr = client->addr;
> + msg[0].flags = 0;
> + msg[0].len = 1;
> + msg[0].buf = &addr_buf;
> +
> + msg[1].addr = client->addr;
> + msg[1].flags = I2C_M_RD;
> + msg[1].len = sizeof(buf);
> + msg[1].buf = buf;
> +
> +#define READ_CALIB_BUF(reg) ((u16)buf[((reg) - PRM1_X_H)])
> +
> + reg1_orig = i2c_smbus_read_byte_data(client, CALIBRATION_REG1);
> + if (reg1_orig < 0)
> + return reg1_orig;
> +
> + reg2_orig = i2c_smbus_read_byte_data(client, CALIBRATION_REG2);
> + if (reg2_orig < 0)
> + return reg2_orig;
> +
> + reg3_orig = i2c_smbus_read_byte_data(client, CALIBRATION_REG3);
> + if (reg3_orig < 0)
> + return reg3_orig;
> +
> + ret = i2c_smbus_write_byte_data(client, INT_MASK,
> + COORD_UPDATE | SLEEP_IN | SLEEP_OUT |
> + PROGRAM_LOAD_DONE);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + ret = i2c_smbus_write_byte_data(client, TEST1, DUALTOUCH_STABILIZE_ON);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + for (retry = 0; retry < CALIBRATION_RETRY_MAX; retry++) {
> + /* wait 2 sampling for update */
> + mdelay(2 * SAMPLING_DELAY);
> +
> + ret = rohm_i2c_burst_read(client->adapter, msg, 2);
> + if (ret < 0) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + if (READ_CALIB_BUF(TOUCH) & TOUCH_DETECT)
> + continue;
> +
> + if (first_time) {
> + /* generate calibration parameter */
> + calib_x =
> + (READ_CALIB_BUF(PRM1_X_H) << 2 |
> + READ_CALIB_BUF(PRM1_X_L)) - AXIS_OFFSET;
> + calib_y =
> + (READ_CALIB_BUF(PRM1_Y_H) << 2 |
> + READ_CALIB_BUF(PRM1_Y_L)) - AXIS_OFFSET;
> +
> + ret = i2c_smbus_write_byte_data(client, TEST1,
> + DUALTOUCH_STABILIZE_ON |
> + DUALTOUCH_REG_ON);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + first_time = false;
> + } else {
> + /* generate adjustment parameter */
> + err_x = READ_CALIB_BUF(PRM1_X_H) << 2 |
> + READ_CALIB_BUF(PRM1_X_L);
> + err_y = READ_CALIB_BUF(PRM1_Y_H) << 2 |
> + READ_CALIB_BUF(PRM1_Y_L);
> +
> + /* X axis ajust */
> + if (err_x <= 4)
> + calib_x -= AXIS_ADJUST;
> + else if (err_x >= 60)
> + calib_x += AXIS_ADJUST;
> +
> + /* Y axis ajust */
> + if (err_y <= 4)
> + calib_y -= AXIS_ADJUST;
> + else if (err_y >= 60)
> + calib_y += AXIS_ADJUST;
> + }
> +
> + /* generate calibration setting value */
> + reg_x = calib_x + ((calib_x & 0x200) << 1);
> + reg_y = calib_y + ((calib_y & 0x200) << 1);
> +
> + /* convert for register format */
> + reg1 = reg_x >> 3;
> + reg2 = (reg_y & 0x7) << 4 | (reg_x & 0x7);
> + reg3 = reg_y >> 3;
> +
> + ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG1, reg1);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG2, reg2);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG3, reg3);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + /*
> + * force calibration sequcence
> + */
> + ret = i2c_smbus_write_byte_data(client, FORCE_CALIBRATION,
> + FORCE_CALIBRATION_OFF);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + ret = i2c_smbus_write_byte_data(client, FORCE_CALIBRATION,
> + FORCE_CALIBRATION_ON);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + /* clear all interrupts */
> + ret = i2c_smbus_write_byte_data(client, INT_CLEAR, 0xff);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + /*
> + * Wait for the status change of calibration, max 10 sampling
> + */
> + calibration_done = false;
> +
> + for (i = 0; i < 10; i++) {
> + mdelay(SAMPLING_DELAY);
> +
> + val = i2c_smbus_read_byte_data(client, TOUCH_GESTURE);
> + if (!(val & CALIBRATION_MASK)) {
> + calibration_done = true;
> + break;
> + } else if (val < 0) {
> + err = val;
> + goto err_exit;
> + }
> + }
> +
> + if (calibration_done) {
> + val = i2c_smbus_read_byte_data(client, INT_STATUS);
> + if (val == CALIBRATION_DONE) {
> + success = true;
> + break;
> + } else if (val < 0) {
> + err = val;
> + goto err_exit;
> + }
> + } else
> + dev_warn(dev, "Calibration timeout\n");
Curly braces around this branch too please.
> + }
> +
> + if (!success) {
> + ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG1,
> + reg1_orig);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG2,
> + reg2_orig);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG3,
> + reg3_orig);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + /* calibration data enable */
> + ret = i2c_smbus_write_byte_data(client, TEST1,
> + DUALTOUCH_STABILIZE_ON |
> + DUALTOUCH_REG_ON);
> + if (ret) {
> + err = ret;
> + goto err_exit;
> + }
> +
> + /* wait 10 sampling */
> + mdelay(10 * SAMPLING_DELAY);
> +
> + err = -EBUSY;
> + }
> +
> +err_exit:
> + ret = i2c_smbus_write_byte_data(client, INT_MASK, INT_ALL);
> + if (!ret)
> + /* Clear all interrupts */
> + ret = i2c_smbus_write_byte_data(client, INT_CLEAR, 0xff);
> +
> + if (!err && ret)
> + err = ret;
> +
> + return err;
> +}
> +
> +static unsigned long inactive_polling_interval[2] = { 1, 0 };
> +static unsigned long active_polling_interval[2] = { 0, 10000000 };
> +
> +module_param_array(inactive_polling_interval, ulong, NULL, S_IRUGO | S_IWUSR);
> +module_param_array(active_polling_interval, ulong, NULL, S_IRUGO | S_IWUSR);
> +
> +MODULE_PARM_DESC(inactive_polling_interval,
> + "Polling interval for un-touch detection");
> +MODULE_PARM_DESC(active_polling_interval,
> + "Polling interval for touch detection");
> +
> +#define INACTIVE_POLLING_INTERVAL_KTIME \
> + ktime_set(inactive_polling_interval[0], inactive_polling_interval[1])
> +#define ACTIVE_POLLING_INTERVAL_KTIME \
> + ktime_set(active_polling_interval[0], active_polling_interval[1])
I do not see it being used?
> +
> +static unsigned int untouch_threshold[3] = { 0, 1, 5 };
> +static unsigned int single_touch_threshold[3] = { 0, 0, 4 };
> +static unsigned int dual_touch_threshold[3] = { 10, 8, 0 };
> +
> +module_param_array(untouch_threshold, uint, NULL, S_IRUGO | S_IWUSR);
> +module_param_array(single_touch_threshold, uint, NULL, S_IRUGO | S_IWUSR);
> +module_param_array(dual_touch_threshold, uint, NULL, S_IRUGO | S_IWUSR);
> +
> +MODULE_PARM_DESC(untouch_threshold, "Thresholds for un-touch detection");
> +MODULE_PARM_DESC(single_touch_threshold,
> + "Thresholds for single touch detection");
> +MODULE_PARM_DESC(dual_touch_threshold, "Thresholds for dual touch detection");
> +
> +static irqreturn_t rohm_ts_soft_irq(int irq, void *dev_id)
> +{
> + struct rohm_ts_data *ts = dev_id;
> + struct i2c_client *client = ts->client;
> + struct input_dev *input_dev = ts->input_dev;
> + struct device *dev = &client->dev;
> +
> + struct i2c_msg msg[2];
> + u16 addr = client->addr;
> + u8 addr_buf;
> + u8 buf[10]; /* for 0x20-0x29 */
> +
> + struct input_mt_pos pos[BU21023_MAX_SLOTS];
> + int slots[BU21023_MAX_SLOTS];
> + u8 touch_flags;
> + unsigned int threshold;
> + int touch_report = -1;
> + unsigned int prev_touch_report = ts->prev_touch_report;
> +
> + s32 status;
> + int i, ret;
> +
> + status = i2c_smbus_read_byte_data(client, INT_STATUS);
> + if (!status)
> + return IRQ_NONE;
> +
> + if (status < 0)
> + return IRQ_HANDLED;
> +
> + ret = i2c_smbus_write_byte_data(client, INT_MASK, INT_ALL);
> + if (ret)
> + return IRQ_HANDLED;
> +
> + /* Clear all interrupts */
> + ret = i2c_smbus_write_byte_data(client, INT_CLEAR, 0xff);
> + if (ret)
> + return IRQ_HANDLED;
> +
> + addr_buf = POS_X1_H;
> + msg[0].addr = addr;
> + msg[0].flags = 0;
> + msg[0].len = 1;
> + msg[0].buf = &addr_buf;
> +
> + msg[1].addr = addr;
> + msg[1].flags = I2C_M_RD;
> + msg[1].len = sizeof(buf);
> + msg[1].buf = buf;
> +
> +#define READ_POS_BUF(reg) ((u16)buf[((reg) - POS_X1_H)])
> +
> + ret = rohm_i2c_burst_read(client->adapter, msg, 2);
> + if (ret < 0)
> + return IRQ_HANDLED;
> +
> + touch_flags = READ_POS_BUF(TOUCH_GESTURE) & TOUCH_MASK;
> + if (touch_flags) {
> + /* generate coordinates */
> + pos[0].x = (READ_POS_BUF(POS_X1_H) << 2) |
> + READ_POS_BUF(POS_X1_L);
> + pos[0].y = (READ_POS_BUF(POS_Y1_H) << 2) |
> + READ_POS_BUF(POS_Y1_L);
> + pos[1].x = (READ_POS_BUF(POS_X2_H) << 2) |
> + READ_POS_BUF(POS_X2_L);
> + pos[1].y = (READ_POS_BUF(POS_Y2_H) << 2) |
> + READ_POS_BUF(POS_Y2_L);
> +
> + switch (touch_flags) {
> + case SINGLE_TOUCH:
> + ts->untouch_count = 0;
> + ts->single_touch_count++;
> + ts->dual_touch_count = 0;
> +
> + threshold = single_touch_threshold[prev_touch_report];
> + if (ts->single_touch_count > threshold)
> + touch_report = 1;
> +
> + if (touch_report == 1) {
> + if (pos[1].x != 0 && pos[1].y != 0) {
> + pos[0].x = pos[1].x;
> + pos[0].y = pos[1].y;
> + pos[1].x = 0;
> + pos[1].y = 0;
> + }
> + }
> + break;
> + case DUAL_TOUCH:
> + ts->untouch_count = 0;
> + ts->single_touch_count = 0;
> + ts->dual_touch_count++;
> +
> + threshold = dual_touch_threshold[prev_touch_report];
> + if (ts->dual_touch_count > threshold)
> + touch_report = 2;
> + break;
> + default:
> + dev_err(dev,
> + "Three or more touches are not supported\n");
You do not want to pollute logs with this, please use dev_dbg().
> + return IRQ_HANDLED;
> + break;
> + }
> + } else {
> + ts->untouch_count++;
> +
> + threshold = untouch_threshold[prev_touch_report];
I do nto understand the therhold logic. You have an array but never
update prev_touch_report index so it is always 0... And why do you need
count at all?
> + if (ts->untouch_count > threshold) {
> + ts->untouch_count = 0;
> + ts->single_touch_count = 0;
> + ts->dual_touch_count = 0;
> +
> + touch_report = 0;
> + }
> + }
> +
> + input_mt_assign_slots(input_dev, slots, pos, touch_report);
> +
> + for (i = 0; i < touch_report; i++) {
> + input_mt_slot(input_dev, slots[i]);
> + input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, true);
> + input_report_abs(input_dev, ABS_MT_POSITION_X, pos[i].x);
> + input_report_abs(input_dev, ABS_MT_POSITION_Y, pos[i].y);
> + }
> +
> + input_mt_sync_frame(input_dev);
> + input_mt_report_pointer_emulation(input_dev, true);
> + input_sync(input_dev);
> +
> + if (READ_POS_BUF(TOUCH_GESTURE) & CALIBRATION_REQUEST) {
> + if (rohm_ts_manual_calibration(ts) < 0)
> + dev_warn(dev, "Failed to manual calibration\n");
> + }
> +
> +
> + i2c_smbus_write_byte_data(client, INT_MASK,
> + CALIBRATION_DONE | SLEEP_OUT | SLEEP_IN |
> + PROGRAM_LOAD_DONE);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t rohm_ts_hard_irq(int irq, void *dev_id)
> +{
> + return IRQ_WAKE_THREAD;
> +}
No need for this function, you can just pass NULL to request_threaded_irq().
> +
> +static int rohm_ts_load_firmware(struct i2c_client *client,
> + const char *firmware_name)
> +{
> + struct device *dev = &client->dev;
> + const struct firmware *firmware = NULL;
> + s32 status;
> + int blocks, remainder, retry = 0, offset;
> + int err = 0, ret;
> + int i;
> +
> + ret = request_firmware(&firmware, firmware_name, dev);
> + if (ret) {
> + dev_err(dev, "Unable to open firmware %s\n", firmware_name);
> + return ret;
> + }
> +
> + blocks = firmware->size / FIRMWARE_BLOCK_SIZE;
> + remainder = firmware->size % FIRMWARE_BLOCK_SIZE;
> +
> + ret = i2c_smbus_write_byte_data(client, INT_MASK,
> + COORD_UPDATE | CALIBRATION_DONE |
> + SLEEP_IN | SLEEP_OUT);
> + if (ret) {
> + err = ret;
> + goto err_int_mask_exit;
> + }
> +
> + while (retry < FIRMWARE_RETRY_MAX) {
> + ret = i2c_smbus_write_byte_data(client, COMMON_SETUP1,
> + COMMON_SETUP1_DEFAULT);
> + if (ret) {
> + err = ret;
> + goto err_int_mask_exit;
> + }
> +
> + ret = i2c_smbus_write_byte_data(client, EX_ADDR_H, 0);
> + if (ret) {
> + err = ret;
> + goto err_int_mask_exit;
> + }
> +
> + ret = i2c_smbus_write_byte_data(client, EX_ADDR_L, 0);
> + if (ret) {
> + err = ret;
> + goto err_int_mask_exit;
> + }
> +
> + offset = 0;
> +
> + /* firmware load to the device */
> + for (i = 0; i < blocks; i++) {
> + ret = i2c_smbus_write_i2c_block_data(client, EX_WDAT,
> + FIRMWARE_BLOCK_SIZE, &firmware->data[offset]);
> + if (ret) {
> + err = ret;
> + goto err_int_mask_exit;
> + }
> +
> + offset += FIRMWARE_BLOCK_SIZE;
> + }
> +
> + if (remainder) {
> + ret = i2c_smbus_write_i2c_block_data(client, EX_WDAT,
> + remainder, &firmware->data[offset]);
> + if (ret) {
> + err = ret;
> + goto err_int_mask_exit;
> + }
> + }
> +
> + /* check formware load result */
> + status = i2c_smbus_read_byte_data(client, INT_STATUS);
> + if (status < 0) {
> + err = status;
> + goto err_int_mask_exit;
> + }
> +
> + /* clear all interrupts */
> + ret = i2c_smbus_write_byte_data(client, INT_CLEAR, 0xff);
> + if (ret) {
> + err = ret;
> + goto err_int_mask_exit;
> + }
> +
> + if (status == PROGRAM_LOAD_DONE)
> + break;
> +
> + /* settings for retry */
> + ret = i2c_smbus_write_byte_data(client, EX_WDAT, 0);
> + if (ret) {
> + err = ret;
> + goto err_int_mask_exit;
> + }
> +
> + retry++;
> + dev_warn(dev, "Retry firmware load\n");
> + }
> +
> +err_int_mask_exit:
> + ret = i2c_smbus_write_byte_data(client, INT_MASK, INT_ALL);
> + if (ret)
> + err = ret;
> +
> + release_firmware(firmware);
> +
> + if (retry >= FIRMWARE_RETRY_MAX)
> + return -EBUSY;
> +
> + return err;
> +}
> +
> +static bool swap_xy;
> +module_param(swap_xy, bool, S_IRUGO);
> +MODULE_PARM_DESC(swap_xy, "Swap X-axis and Y-axis");
> +
> +static bool inv_x;
> +module_param(inv_x, bool, S_IRUGO);
> +MODULE_PARM_DESC(inv_x, "Invert X-axis");
> +
> +static bool inv_y;
> +module_param(inv_y, bool, S_IRUGO);
> +MODULE_PARM_DESC(inv_y, "Invert Y-axis");
This should be per-device parameters, not module parameters.
> +
> +static int rohm_ts_device_init(struct rohm_ts_data *ts)
> +{
> + struct i2c_client *client = ts->client;
> + struct device *dev = &client->dev;
> + u8 val;
> + int ret;
> +
> + /*
> + * Wait 200usec for reset
> + */
> + udelay(200);
> +
> + /* Release analog reset */
> + ret = i2c_smbus_write_byte_data(client, SYSTEM, ANALOG_POWER_ON);
> + if (ret)
> + return ret;
> +
> + /* Waiting for the analog warm-up, max. 200usec */
> + udelay(200);
> +
> + /* clear all interrupts */
> + ret = i2c_smbus_write_byte_data(client, INT_CLEAR, 0xff);
> + if (ret)
> + return ret;
> +
> + val = MAF_1SAMPLE;
> + if (swap_xy)
> + val |= SWAP_XY;
> + if (inv_x)
> + val |= INV_X;
> + if (inv_y)
> + val |= INV_Y;
> +
> + ret = i2c_smbus_write_byte_data(client, EX_WDAT, 0);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, COMMON_SETUP1, 0);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, COMMON_SETUP2, val);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, COMMON_SETUP3,
> + SEL_TBL_DEFAULT | EN_MULTI);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, THRESHOLD_GESTURE,
> + THRESHOLD_GESTURE_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, INTERVAL_TIME,
> + INTERVAL_TIME_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, CPU_FREQ, CPU_FREQ_10MHZ);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, PRM_SWOFF_TIME,
> + PRM_SWOFF_TIME_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, ADC_CTRL, ADC_DIV_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, ADC_WAIT, ADC_WAIT_DEFAULT);
> + if (ret)
> + return ret;
> +
> + /*
> + * Panel setup, these values change with the panel.
> + */
> + ret = i2c_smbus_write_byte_data(client, STEP_X, STEP_X_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, STEP_Y, STEP_Y_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, OFFSET_X, OFFSET_X_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, OFFSET_Y, OFFSET_Y_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, THRESHOLD_TOUCH,
> + THRESHOLD_TOUCH_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, EVR_XY, EVR_XY_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, EVR_X, EVR_X_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, EVR_Y, EVR_Y_DEFAULT);
> + if (ret)
> + return ret;
> +
> + /* Fixed value settings */
> + ret = i2c_smbus_write_byte_data(client, CALIBRATION_ADJUST,
> + CALIBRATION_ADJUST_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, SWCONT, SWCONT_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, TEST1,
> + DUALTOUCH_STABILIZE_ON |
> + DUALTOUCH_REG_ON);
> + if (ret)
> + return ret;
> +
> + ret = rohm_ts_load_firmware(client, BU21023_FIRMWARE_NAME);
> + if (ret) {
> + dev_err(dev, "Failed to firmware load\n");
> + return ret;
> + }
> +
> + /*
> + * Manual calibration results are not changed in same environment.
> + * If the force calibration is performed,
> + * the controller will not require calibration request interrupt
> + * when the typical values are set to the calibration registers.
> + */
> + ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG1,
> + CALIBRATION_REG1_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG2,
> + CALIBRATION_REG2_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, CALIBRATION_REG3,
> + CALIBRATION_REG3_DEFAULT);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, FORCE_CALIBRATION,
> + FORCE_CALIBRATION_OFF);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, FORCE_CALIBRATION,
> + FORCE_CALIBRATION_ON);
> + if (ret)
> + return ret;
> +
> + /* Clear all interrupts */
> + ret = i2c_smbus_write_byte_data(client, INT_CLEAR, 0xff);
> + if (ret)
> + return ret;
> +
> + ret = devm_request_threaded_irq(dev, client->irq, rohm_ts_hard_irq,
> + rohm_ts_soft_irq, IRQF_TRIGGER_FALLING,
> + client->name, ts);
> + if (ret) {
> + dev_err(dev, "Unable to request IRQ\n");
> + return ret;
> + }
> +
> + /* Enable coordinates update interrupt */
> + ret = i2c_smbus_write_byte_data(client, INT_MASK,
> + CALIBRATION_DONE |
> + SLEEP_OUT | SLEEP_IN |
> + PROGRAM_LOAD_DONE);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, ERR_MASK,
> + PROGRAM_LOAD_ERR | CPU_TIMEOUT |
> + ADC_TIMEOUT);
> + if (ret)
> + return ret;
> +
> + /* controller CPU power on */
> + ret = i2c_smbus_write_byte_data(client, SYSTEM,
> + CPU_POWER_ON | ANALOG_POWER_ON);
> +
> + return ret;
> +}
> +
> +static int rohm_ts_open(struct input_dev *input_dev)
> +{
> + struct rohm_ts_data *ts = input_get_drvdata(input_dev);
> + struct i2c_client *client = ts->client;
> + int ret;
> +
> + if (!ts->initialized) {
> + ret = rohm_ts_device_init(ts);
> + if (ret) {
> + dev_err(&client->dev,
> + "Failed to device initialization\n");
> + return ret;
> + }
> +
> + ts->initialized = true;
> + }
> +
> + return 0;
> +}
> +
> +static int rohm_bu21023_i2c_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct rohm_ts_data *ts;
> + struct device *dev = &client->dev;
> + struct input_dev *input_dev;
> + int ret;
> +
> + if (!client->irq) {
> + dev_err(dev, "IRQ is not assigned\n");
> + return -EBUSY;
-EINVAL. There is no point in retrying.
> + }
> +
> + ts = devm_kzalloc(dev, sizeof(struct rohm_ts_data), GFP_KERNEL);
> + if (!ts)
> + return -ENOMEM;
> +
> + ts->client = client;
> + i2c_set_clientdata(client, ts);
> +
> + input_dev = devm_input_allocate_device(dev);
> + if (!input_dev)
> + return -ENOMEM;
> +
> + input_dev->name = BU21023_NAME;
> + input_dev->id.bustype = BUS_I2C;
> + input_dev->open = rohm_ts_open;
> +
> + ts->input_dev = input_dev;
> + input_set_drvdata(input_dev, ts);
> +
> + set_bit(EV_SYN, input_dev->evbit);
> + set_bit(EV_KEY, input_dev->evbit);
> + set_bit(EV_ABS, input_dev->evbit);
> +
> + set_bit(BTN_TOUCH, input_dev->evbit);
> +
> + input_set_abs_params(input_dev, ABS_MT_POSITION_X,
> + ROHM_TS_ABS_X_MIN, ROHM_TS_ABS_X_MAX, 0, 0);
> + input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
> + ROHM_TS_ABS_Y_MIN, ROHM_TS_ABS_Y_MAX, 0, 0);
> +
> + input_set_abs_params(input_dev, ABS_X,
> + ROHM_TS_ABS_X_MIN, ROHM_TS_ABS_X_MAX, 0, 0);
> + input_set_abs_params(input_dev, ABS_Y,
> + ROHM_TS_ABS_Y_MIN, ROHM_TS_ABS_Y_MAX, 0, 0);
> +
> + ret = input_register_device(input_dev);
> + if (ret)
> + dev_err(dev, "Unable to register input device\n");
> +
> + return ret;
> +}
> +
> +static int rohm_bu21023_i2c_remove(struct i2c_client *client)
> +{
> + struct rohm_ts_data *ts = i2c_get_clientdata(client);
> + int ret;
> +
> + input_unregister_device(ts->input_dev);
> +
> + ret = i2c_smbus_write_byte_data(client, SYSTEM,
> + ANALOG_POWER_ON | CPU_POWER_OFF);
> + if (ret)
> + return ret;
> +
> + ret = i2c_smbus_write_byte_data(client, SYSTEM,
> + ANALOG_POWER_OFF | CPU_POWER_OFF);
> +
> + return ret;
> +}
> +
> +static const struct i2c_device_id rohm_bu21023_i2c_id[] = {
> + {BU21023_NAME, 0},
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(i2c, rohm_bu21023_i2c_id);
> +
> +static struct i2c_driver rohm_bu21023_i2c_driver = {
> + .driver = {
> + .name = BU21023_NAME,
> + },
This is weird formatting.
> + .probe = rohm_bu21023_i2c_probe,
> + .remove = rohm_bu21023_i2c_remove,
> + .id_table = rohm_bu21023_i2c_id,
> +};
> +
> +module_i2c_driver(rohm_bu21023_i2c_driver);
> +
> +MODULE_DESCRIPTION("ROHM BU21023/24 Touchscreen driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("ROHM Co., Ltd.");
> diff --git a/drivers/input/touchscreen/rohm_bu21023.h b/drivers/input/touchscreen/rohm_bu21023.h
> new file mode 100644
> index 0000000..302581b
> --- /dev/null
> +++ b/drivers/input/touchscreen/rohm_bu21023.h
> @@ -0,0 +1,258 @@
> +/*
> + * ROHM BU21023/24 Dual touch support resistive touch screen driver
> + * Copyright (C) 2012 ROHM CO.,LTD.
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * 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 _BU21023_TS_H
> +#define _BU21023_TS_H
> +
> +#define BU21023_NAME "bu21023_ts"
> +#define BU21023_FIRMWARE_NAME "bu21023.bin"
> +
> +#define BU21023_MAX_SLOTS 2
> +
> +#define AXIS_ADJUST 4
> +#define AXIS_OFFSET 8
> +
> +#define FIRMWARE_BLOCK_SIZE 32
> +#define FIRMWARE_RETRY_MAX 4
> +
> +#define SAMPLING_DELAY 12 /* msec */
> +
> +#define CALIBRATION_RETRY_MAX 6
> +
> +#define ROHM_TS_ABS_X_MIN 40
> +#define ROHM_TS_ABS_X_MAX 990
> +#define ROHM_TS_ABS_Y_MIN 160
> +#define ROHM_TS_ABS_Y_MAX 920
> +
> +/*
> + * BU21023GUL/BU21023MUV/BU21024FV-M registers map
> + */
> +#define VADOUT_YP_H 0x00
> +#define VADOUT_YP_L 0x01
> +#define VADOUT_XP_H 0x02
> +#define VADOUT_XP_L 0x03
> +#define VADOUT_YN_H 0x04
> +#define VADOUT_YN_L 0x05
> +#define VADOUT_XN_H 0x06
> +#define VADOUT_XN_L 0x07
> +
> +#define PRM1_X_H 0x08
> +#define PRM1_X_L 0x09
> +#define PRM1_Y_H 0x0a
> +#define PRM1_Y_L 0x0b
> +#define PRM2_X_H 0x0c
> +#define PRM2_X_L 0x0d
> +#define PRM2_Y_H 0x0e
> +#define PRM2_Y_L 0x0f
> +
> +#define MLT_PRM_MONI_X 0x10
> +#define MLT_PRM_MONI_Y 0x11
> +
> +#define DEBUG_MONI_1 0x12
> +#define DEBUG_MONI_2 0x13
> +
> +#define VADOUT_ZX_H 0x14
> +#define VADOUT_ZX_L 0x15
> +#define VADOUT_ZY_H 0x16
> +#define VADOUT_ZY_L 0x17
> +
> +#define Z_PARAM_H 0x18
> +#define Z_PARAM_L 0x19
> +
> +/*
> + * Value for VADOUT_*_L
> + */
> +#define VADOUT_L_MASK 0x01
> +
> +/*
> + * Value for PRM*_*_L
> + */
> +#define PRM_L_MASK 0x01
> +
> +#define POS_X1_H 0x20
> +#define POS_X1_L 0x21
> +#define POS_Y1_H 0x22
> +#define POS_Y1_L 0x23
> +#define POS_X2_H 0x24
> +#define POS_X2_L 0x25
> +#define POS_Y2_H 0x26
> +#define POS_Y2_L 0x27
> +
> +/*
> + * Value for POS_*_L
> + */
> +#define POS_L_MASK 0x01
> +
> +#define TOUCH 0x28
> +#define TOUCH_DETECT 0x01
> +
> +#define TOUCH_GESTURE 0x29
> +#define SINGLE_TOUCH 0x01
> +#define DUAL_TOUCH 0x03
> +#define TOUCH_MASK 0x03
> +#define CALIBRATION_REQUEST 0x04
> +#define CALIBRATION_STATUS 0x08
> +#define CALIBRATION_MASK 0x0c
> +#define GESTURE_SPREAD 0x10
> +#define GESTURE_PINCH 0x20
> +#define GESTURE_ROTATE_R 0x40
> +#define GESTURE_ROTATE_L 0x80
> +
> +#define INT_STATUS 0x2a
> +#define INT_MASK 0x3d
> +#define INT_CLEAR 0x3e
> +
> +/*
> + * Values for INT_*
> + */
> +#define COORD_UPDATE 0x01
> +#define CALIBRATION_DONE 0x02
> +#define SLEEP_IN 0x04
> +#define SLEEP_OUT 0x08
> +#define PROGRAM_LOAD_DONE 0x10
> +#define ERROR 0x80
> +#define INT_ALL 0x9f
> +
> +#define ERR_STATUS 0x2b
> +#define ERR_MASK 0x3f
> +
> +/*
> + * Values for ERR_*
> + */
> +#define ADC_TIMEOUT 0x01
> +#define CPU_TIMEOUT 0x02
> +#define CALIBRATION_ERR 0x04
> +#define PROGRAM_LOAD_ERR 0x10
> +
> +#define COMMON_SETUP1 0x30
> +#define PROGRAM_LOAD_HOST 0x02
> +#define PROGRAM_LOAD_EEPROM 0x03
> +#define CENSOR_4PORT 0x04
> +#define CENSOR_8PORT 0x00 /* Not supported by BU21023 */
> +#define CALIBRATION_TYPE_DEFAULT 0x08
> +#define CALIBRATION_TYPE_SPECIAL 0x00
> +#define INT_ACTIVE_HIGH 0x10
> +#define INT_ACTIVE_LOW 0x00
> +#define AUTO_CALIBRATION 0x40
> +#define MANUAL_CALIBRATION 0x00
> +#define COMMON_SETUP1_DEFAULT 0x4e
> +
> +#define COMMON_SETUP2 0x31
> +#define MAF_NONE 0x00
> +#define MAF_1SAMPLE 0x01
> +#define MAF_3SAMPLES 0x02
> +#define MAF_5SAMPLES 0x03
> +#define INV_Y 0x04
> +#define INV_X 0x08
> +#define SWAP_XY 0x10
> +
> +#define COMMON_SETUP3 0x32
> +#define EN_SLEEP 0x01
> +#define EN_MULTI 0x02
> +#define EN_GESTURE 0x04
> +#define EN_INTVL 0x08
> +#define SEL_STEP 0x10
> +#define SEL_MULTI 0x20
> +#define SEL_TBL_DEFAULT 0x40
> +
> +#define INTERVAL_TIME 0x33
> +#define INTERVAL_TIME_DEFAULT 0x10
> +
> +#define STEP_X 0x34
> +#define STEP_X_DEFAULT 0x41
> +
> +#define STEP_Y 0x35
> +#define STEP_Y_DEFAULT 0x8d
> +
> +#define OFFSET_X 0x38
> +#define OFFSET_X_DEFAULT 0x0c
> +
> +#define OFFSET_Y 0x39
> +#define OFFSET_Y_DEFAULT 0x0c
> +
> +#define THRESHOLD_TOUCH 0x3a
> +#define THRESHOLD_TOUCH_DEFAULT 0xa0
> +
> +#define THRESHOLD_GESTURE 0x3b
> +#define THRESHOLD_GESTURE_DEFAULT 0x17
> +
> +#define SYSTEM 0x40
> +#define ANALOG_POWER_ON 0x01
> +#define ANALOG_POWER_OFF 0x00
> +#define CPU_POWER_ON 0x02
> +#define CPU_POWER_OFF 0x00
> +
> +#define FORCE_CALIBRATION 0x42
> +#define FORCE_CALIBRATION_ON 0x01
> +#define FORCE_CALIBRATION_OFF 0x00
> +
> +#define CPU_FREQ 0x50 /* 10 / (reg + 1) MHz */
> +#define CPU_FREQ_10MHZ 0x00
> +#define CPU_FREQ_5MHZ 0x01
> +#define CPU_FREQ_1MHZ 0x09
> +
> +#define EEPROM_ADDR 0x51
> +
> +#define CALIBRATION_ADJUST 0x52
> +#define CALIBRATION_ADJUST_DEFAULT 0x00
> +
> +#define THRESHOLD_SLEEP_IN 0x53
> +
> +#define EVR_XY 0x56
> +#define EVR_XY_DEFAULT 0x10
> +
> +#define PRM_SWOFF_TIME 0x57
> +#define PRM_SWOFF_TIME_DEFAULT 0x04
> +
> +#define PROGRAM_VERSION 0x5f
> +
> +#define ADC_CTRL 0x60
> +#define ADC_DIV_MASK 0x1f /* The minimum value is 4 */
> +#define ADC_DIV_DEFAULT 0x08
> +
> +#define ADC_WAIT 0x61
> +#define ADC_WAIT_DEFAULT 0x0a
> +
> +#define SWCONT 0x62
> +#define SWCONT_DEFAULT 0x0f
> +
> +#define EVR_X 0x63
> +#define EVR_X_DEFAULT 0x86
> +
> +#define EVR_Y 0x64
> +#define EVR_Y_DEFAULT 0x64
> +
> +#define TEST1 0x65
> +#define DUALTOUCH_STABILIZE_ON 0x01
> +#define DUALTOUCH_STABILIZE_OFF 0x00
> +#define DUALTOUCH_REG_ON 0x20
> +#define DUALTOUCH_REG_OFF 0x00
> +
> +#define CALIBRATION_REG1 0x68
> +#define CALIBRATION_REG1_DEFAULT 0xd9
> +
> +#define CALIBRATION_REG2 0x69
> +#define CALIBRATION_REG2_DEFAULT 0x36
> +
> +#define CALIBRATION_REG3 0x6a
> +#define CALIBRATION_REG3_DEFAULT 0x32
> +
> +#define EX_ADDR_H 0x70
> +#define EX_ADDR_L 0x71
> +#define EX_WDAT 0x72
> +#define EX_RDAT 0x73
> +#define EX_CHK_SUM1 0x74
> +#define EX_CHK_SUM2 0x75
> +#define EX_CHK_SUM3 0x76
> +
> +#endif /* _BU21023_TS_H */
> --
> 1.7.9.5
>
Why does this have to be in a separate header if there are no users of
it?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v2 0/3] psmouse: Add support for detecting FocalTech PS/2
From: Hans de Goede @ 2014-09-12 9:08 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Peter Hutterer, Benjamin Tissoires, linux-input
Hi Dmitry,
Here is v2 of my patch-set to "deal" with focaltech touchpads. As requested
in this version I'm doing the reset from a focaltech_init function, and the
goto has been dropped.
Regards,
Hans
^ permalink raw reply
* [PATCH v2 1/3] i8042: Also store the aux firmware id in multi-plexed aux ports
From: Hans de Goede @ 2014-09-12 9:08 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Peter Hutterer, Benjamin Tissoires, linux-input, Hans de Goede
In-Reply-To: <1410512912-11609-1-git-send-email-hdegoede@redhat.com>
So that firmware-id matching can be used with multiplexed aux ports too.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/input/serio/i8042.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index 3807c3e..f5a98af 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -1254,6 +1254,8 @@ static int __init i8042_create_aux_port(int idx)
} else {
snprintf(serio->name, sizeof(serio->name), "i8042 AUX%d port", idx);
snprintf(serio->phys, sizeof(serio->phys), I8042_MUX_PHYS_DESC, idx + 1);
+ strlcpy(serio->firmware_id, i8042_aux_firmware_id,
+ sizeof(serio->firmware_id));
}
port->serio = serio;
--
2.1.0
^ permalink raw reply related
* [PATCH v2 2/3] psmouse: Add psmouse_matches_pnp_id helper function
From: Hans de Goede @ 2014-09-12 9:08 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Peter Hutterer, Benjamin Tissoires, linux-input, Hans de Goede
In-Reply-To: <1410512912-11609-1-git-send-email-hdegoede@redhat.com>
The matches_pnp_id function from the synaptics driver is useful for other
drivers too. Make it a generic psmouse helper function.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/input/mouse/psmouse-base.c | 14 ++++++++++++++
drivers/input/mouse/psmouse.h | 1 +
drivers/input/mouse/synaptics.c | 17 +++--------------
3 files changed, 18 insertions(+), 14 deletions(-)
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index b4e1f01..02e68c3 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -462,6 +462,20 @@ static int psmouse_poll(struct psmouse *psmouse)
PSMOUSE_CMD_POLL | (psmouse->pktsize << 8));
}
+/*
+ * psmouse_matches_pnp_id - check if psmouse matches one of the passed in ids.
+ */
+bool psmouse_matches_pnp_id(struct psmouse *psmouse, const char * const ids[])
+{
+ int i;
+
+ if (!strncmp(psmouse->ps2dev.serio->firmware_id, "PNP:", 4))
+ for (i = 0; ids[i]; i++)
+ if (strstr(psmouse->ps2dev.serio->firmware_id, ids[i]))
+ return true;
+
+ return false;
+}
/*
* Genius NetMouse magic init.
diff --git a/drivers/input/mouse/psmouse.h b/drivers/input/mouse/psmouse.h
index 2f0b39d..f4cf664 100644
--- a/drivers/input/mouse/psmouse.h
+++ b/drivers/input/mouse/psmouse.h
@@ -108,6 +108,7 @@ void psmouse_set_resolution(struct psmouse *psmouse, unsigned int resolution);
psmouse_ret_t psmouse_process_byte(struct psmouse *psmouse);
int psmouse_activate(struct psmouse *psmouse);
int psmouse_deactivate(struct psmouse *psmouse);
+bool psmouse_matches_pnp_id(struct psmouse *psmouse, const char * const ids[]);
struct psmouse_attribute {
struct device_attribute dattr;
diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index e8573c6..854caca 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -185,18 +185,6 @@ static const char * const topbuttonpad_pnp_ids[] = {
NULL
};
-static bool matches_pnp_id(struct psmouse *psmouse, const char * const ids[])
-{
- int i;
-
- if (!strncmp(psmouse->ps2dev.serio->firmware_id, "PNP:", 4))
- for (i = 0; ids[i]; i++)
- if (strstr(psmouse->ps2dev.serio->firmware_id, ids[i]))
- return true;
-
- return false;
-}
-
/*****************************************************************************
* Synaptics communications functions
****************************************************************************/
@@ -362,7 +350,8 @@ static int synaptics_resolution(struct psmouse *psmouse)
}
for (i = 0; min_max_pnpid_table[i].pnp_ids; i++) {
- if (matches_pnp_id(psmouse, min_max_pnpid_table[i].pnp_ids)) {
+ if (psmouse_matches_pnp_id(psmouse,
+ min_max_pnpid_table[i].pnp_ids)) {
priv->x_min = min_max_pnpid_table[i].x_min;
priv->x_max = min_max_pnpid_table[i].x_max;
priv->y_min = min_max_pnpid_table[i].y_min;
@@ -1456,7 +1445,7 @@ static void set_input_params(struct psmouse *psmouse,
if (SYN_CAP_CLICKPAD(priv->ext_cap_0c)) {
__set_bit(INPUT_PROP_BUTTONPAD, dev->propbit);
- if (matches_pnp_id(psmouse, topbuttonpad_pnp_ids))
+ if (psmouse_matches_pnp_id(psmouse, topbuttonpad_pnp_ids))
__set_bit(INPUT_PROP_TOPBUTTONPAD, dev->propbit);
/* Clickpads report only left button */
__clear_bit(BTN_RIGHT, dev->keybit);
--
2.1.0
^ permalink raw reply related
* [PATCH v2 3/3] psmouse: Add support for detecting FocalTech PS/2 touchpads
From: Hans de Goede @ 2014-09-12 9:08 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Peter Hutterer, Benjamin Tissoires, linux-input, Hans de Goede
In-Reply-To: <1410512912-11609-1-git-send-email-hdegoede@redhat.com>
The Asus X450 and X550 laptops use a PS/2 touchpad from a new manufacturer
called FocalTech:
https://bugzilla.kernel.org/show_bug.cgi?id=77391
https://bugzilla.redhat.com/show_bug.cgi?id=1110011
The protocol for these devices is not known at this time, but even without
knowing the protocol they need some special handling. They get upset by some
of our other PS/2 device probing, and once upset generate random mouse events
making things unusable even with an external mouse.
This patch adds detection of these devices based on their pnp ids, and when
they are detected, treats them as a bare ps/2 mouse. Doing things this way
they at least work in their ps/2 mouse emulation mode.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/input/mouse/Makefile | 2 +-
drivers/input/mouse/focaltech.c | 52 ++++++++++++++++++++++++++++++++++++++
drivers/input/mouse/focaltech.h | 22 ++++++++++++++++
drivers/input/mouse/psmouse-base.c | 10 ++++++++
4 files changed, 85 insertions(+), 1 deletion(-)
create mode 100644 drivers/input/mouse/focaltech.c
create mode 100644 drivers/input/mouse/focaltech.h
diff --git a/drivers/input/mouse/Makefile b/drivers/input/mouse/Makefile
index c25efdb..dda507f 100644
--- a/drivers/input/mouse/Makefile
+++ b/drivers/input/mouse/Makefile
@@ -23,7 +23,7 @@ obj-$(CONFIG_MOUSE_SYNAPTICS_I2C) += synaptics_i2c.o
obj-$(CONFIG_MOUSE_SYNAPTICS_USB) += synaptics_usb.o
obj-$(CONFIG_MOUSE_VSXXXAA) += vsxxxaa.o
-psmouse-objs := psmouse-base.o synaptics.o
+psmouse-objs := psmouse-base.o synaptics.o focaltech.o
psmouse-$(CONFIG_MOUSE_PS2_ALPS) += alps.o
psmouse-$(CONFIG_MOUSE_PS2_ELANTECH) += elantech.o
diff --git a/drivers/input/mouse/focaltech.c b/drivers/input/mouse/focaltech.c
new file mode 100644
index 0000000..f4d657e
--- /dev/null
+++ b/drivers/input/mouse/focaltech.c
@@ -0,0 +1,52 @@
+/*
+ * Focaltech TouchPad PS/2 mouse driver
+ *
+ * Copyright (c) 2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Red Hat authors:
+ *
+ * Hans de Goede <hdegoede@redhat.com>
+ */
+
+/*
+ * The Focaltech PS/2 touchpad protocol is unknown. This drivers deals with
+ * detection only, to avoid further detection attempts confusing the touchpad
+ * this way it at least works in PS/2 mouse compatibility mode.
+ */
+
+#include <linux/device.h>
+#include <linux/libps2.h>
+#include "psmouse.h"
+
+static const char * const focaltech_pnp_ids[] = {
+ "FLT0101",
+ "FLT0102",
+ "FLT0103",
+ NULL
+};
+
+int focaltech_detect(struct psmouse *psmouse, bool set_properties)
+{
+ if (!psmouse_matches_pnp_id(psmouse, focaltech_pnp_ids))
+ return -ENODEV;
+
+ if (set_properties) {
+ psmouse->vendor = "FocalTech";
+ psmouse->name = "FocalTech Touchpad in mouse emulation mode";
+ }
+
+ return 0;
+}
+
+int focaltech_init(struct psmouse *psmouse)
+{
+ ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_RESET_DIS);
+ psmouse_reset(psmouse);
+
+ return 0;
+}
diff --git a/drivers/input/mouse/focaltech.h b/drivers/input/mouse/focaltech.h
new file mode 100644
index 0000000..498650c
--- /dev/null
+++ b/drivers/input/mouse/focaltech.h
@@ -0,0 +1,22 @@
+/*
+ * Focaltech TouchPad PS/2 mouse driver
+ *
+ * Copyright (c) 2014 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * Red Hat authors:
+ *
+ * Hans de Goede <hdegoede@redhat.com>
+ */
+
+#ifndef _FOCALTECH_H
+#define _FOCALTECH_H
+
+int focaltech_detect(struct psmouse *psmouse, bool set_properties);
+int focaltech_init(struct psmouse *psmouse);
+
+#endif
diff --git a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
index 02e68c3..ae1e76b 100644
--- a/drivers/input/mouse/psmouse-base.c
+++ b/drivers/input/mouse/psmouse-base.c
@@ -35,6 +35,7 @@
#include "elantech.h"
#include "sentelic.h"
#include "cypress_ps2.h"
+#include "focaltech.h"
#define DRIVER_DESC "PS/2 mouse driver"
@@ -722,6 +723,15 @@ static int psmouse_extensions(struct psmouse *psmouse,
{
bool synaptics_hardware = false;
+/* Always check for focaltech, this is safe as it uses pnp-id matching */
+ if (psmouse_do_detect(focaltech_detect, psmouse, set_properties) == 0) {
+ if (!set_properties || focaltech_init(psmouse) == 0) {
+ /* Not supported yet, use bare protocol */
+ psmouse_max_proto = PSMOUSE_PS2;
+ return PSMOUSE_PS2;
+ }
+ }
+
/*
* We always check for lifebook because it does not disturb mouse
* (it only checks DMI information).
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] Handle spurious backslash key repeats on some keyboards
From: David Herrmann @ 2014-09-12 10:29 UTC (permalink / raw)
To: Fredrik Hallenberg; +Cc: Dmitry Torokhov, Jiri Kosina, open list:HID CORE LAYER
In-Reply-To: <CAMsZVf_K1ji75K3CcqGjED01Lppbk8gFSw0ADB-QOG=wVYiF_g@mail.gmail.com>
Hi
On Thu, Sep 11, 2014 at 10:35 PM, Fredrik Hallenberg
<megahallon@gmail.com> wrote:
> Thanks I had no idea about the hwdb but I have tried it now, it works if I use
>
> KEYBOARD_KEY_70031=reserved
Thanks for confirming!
> on my nordic keyboard, but a US keyboard user would have to use
> scancode 70032 instead. If you chose the wrong one the backslash key
> will be disabled.
>
> I don't think this is good solution as long as there is no way to
> detect which version is used with the hardware matcher string.
Well, you can match on "country" tags. Currently, the device
identifiers contain vendor+product+version+country information. I
assume the "country" field is different between your keyboard and a
keyboard that swaps the key reports.
> However I suppose udev should make it possible to do some kind of
> script that does something similar to my original patch, that is,
> detects which of the two keys in actually used and disables the other
> one.
If you post your mod-alias line of your device (or better: all
information you can gather about the device), I can push it into hwdb.
It is up to Dmitry and Jiri to decide whether it makes sense to deal
with it in the kernel.
Thanks
David
^ permalink raw reply
* Re: [PATCH] Handle spurious backslash key repeats on some keyboards
From: Fredrik Hallenberg @ 2014-09-12 11:34 UTC (permalink / raw)
To: David Herrmann; +Cc: Dmitry Torokhov, Jiri Kosina, open list:HID CORE LAYER
In-Reply-To: <CANq1E4Qw3_0M7BCgDJTX0WGHU+CnFdnuASX0JfEK-Btkc=7Rsg@mail.gmail.com>
Is the country tag derived from the bCountryCode seen in the HID
device descriptor seen when doing "lsusb -v"?
If so it probably wont work as it says "0 (Not supported)" on my
keyboard and a few others I have tested.
On Fri, Sep 12, 2014 at 12:29 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Thu, Sep 11, 2014 at 10:35 PM, Fredrik Hallenberg
> <megahallon@gmail.com> wrote:
>> Thanks I had no idea about the hwdb but I have tried it now, it works if I use
>>
>> KEYBOARD_KEY_70031=reserved
>
> Thanks for confirming!
>
>> on my nordic keyboard, but a US keyboard user would have to use
>> scancode 70032 instead. If you chose the wrong one the backslash key
>> will be disabled.
>>
>> I don't think this is good solution as long as there is no way to
>> detect which version is used with the hardware matcher string.
>
> Well, you can match on "country" tags. Currently, the device
> identifiers contain vendor+product+version+country information. I
> assume the "country" field is different between your keyboard and a
> keyboard that swaps the key reports.
>
>> However I suppose udev should make it possible to do some kind of
>> script that does something similar to my original patch, that is,
>> detects which of the two keys in actually used and disables the other
>> one.
>
> If you post your mod-alias line of your device (or better: all
> information you can gather about the device), I can push it into hwdb.
> It is up to Dmitry and Jiri to decide whether it makes sense to deal
> with it in the kernel.
>
> Thanks
> David
^ permalink raw reply
* Re: [PATCH] Handle spurious backslash key repeats on some keyboards
From: David Herrmann @ 2014-09-12 11:47 UTC (permalink / raw)
To: Fredrik Hallenberg; +Cc: Dmitry Torokhov, Jiri Kosina, open list:HID CORE LAYER
In-Reply-To: <CAMsZVf-r6GvPOUEUye9wcLrT-g8SXkk5b3HKBDDTyHjAj38aKQ@mail.gmail.com>
Hi
On Fri, Sep 12, 2014 at 1:34 PM, Fredrik Hallenberg
<megahallon@gmail.com> wrote:
> Is the country tag derived from the bCountryCode seen in the HID
> device descriptor seen when doing "lsusb -v"?
Yes, it is.
> If so it probably wont work as it says "0 (Not supported)" on my
> keyboard and a few others I have tested.
"0" might also be used as default, so it's not surprising that your
US-style keyboard uses "0".
Furthermore, the non-US style keyboards might even use different
product IDs or version numbers. So it'd be nice if you could include a
full set of device information here.
Thanks
David
^ permalink raw reply
* Re: [PATCH] HID: wacom: make the WL connection friendly for the desktop
From: Jiri Kosina @ 2014-09-12 12:16 UTC (permalink / raw)
To: Ping Cheng
Cc: Benjamin Tissoires, Jason Gerecke, linux-input,
linux-kernel@vger.kernel.org
In-Reply-To: <CAF8JNh+U80vPUQ7263cRGFJCZkzsO7nSAg3KOtfOy5Bsp_Xajw@mail.gmail.com>
On Thu, 11 Sep 2014, Ping Cheng wrote:
> > This way, tablets connected through WL can be used from the user point of
> > view in the same way they are used while connected through wire.
> >
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> Reviewed-by: Ping Cheng <pingc@wacom.com>
Applied to for-3.18/wacom.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Handle spurious backslash key repeats on some keyboards
From: Fredrik Hallenberg @ 2014-09-12 17:36 UTC (permalink / raw)
To: David Herrmann; +Cc: Dmitry Torokhov, Jiri Kosina, open list:HID CORE LAYER
In-Reply-To: <CANq1E4Rw7q9QpA1UYsjuu9umB_AE2qE6JhNKQ9e8W3xp+m_2EA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1160 bytes --]
Here is the output from lsusb, let me know if you need more information.
Note this is the nordic version so if they are using the country code
at all it should have some other value than 0. As I mentioned I
checked a few other nordic/swedish keyboards and all of them use code
0.
Anyway as you say there could be some other value that could be used,
I will try to find someone with a US-version of this keyboard and
compare the values.
On Fri, Sep 12, 2014 at 1:47 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Fri, Sep 12, 2014 at 1:34 PM, Fredrik Hallenberg
> <megahallon@gmail.com> wrote:
>> Is the country tag derived from the bCountryCode seen in the HID
>> device descriptor seen when doing "lsusb -v"?
>
> Yes, it is.
>
>> If so it probably wont work as it says "0 (Not supported)" on my
>> keyboard and a few others I have tested.
>
> "0" might also be used as default, so it's not surprising that your
> US-style keyboard uses "0".
>
> Furthermore, the non-US style keyboards might even use different
> product IDs or version numbers. So it'd be nice if you could include a
> full set of device information here.
>
> Thanks
> David
[-- Attachment #2: corsair_k70.txt --]
[-- Type: text/plain, Size: 4187 bytes --]
Bus 001 Device 003: ID 1b1c:1b09 Corsair
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 0 (Defined at Interface level)
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 64
idVendor 0x1b1c Corsair
idProduct 0x1b09
bcdDevice 1.09
iManufacturer 1 Corsair
iProduct 2 Corsair K70R Gaming Keyboard
iSerial 0
bNumConfigurations 1
Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength 84
bNumInterfaces 3
bConfigurationValue 1
iConfiguration 0
bmAttributes 0xa0
(Bus Powered)
Remote Wakeup
MaxPower 500mA
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 0
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 3 Human Interface Device
bInterfaceSubClass 1 Boot Interface Subclass
bInterfaceProtocol 1 Keyboard
iInterface 0
HID Device Descriptor:
bLength 9
bDescriptorType 33
bcdHID 1.11
bCountryCode 0 Not supported
bNumDescriptors 1
bDescriptorType 34 Report
wDescriptorLength 65
Report Descriptors:
** UNAVAILABLE **
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0008 1x 8 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 1
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 3 Human Interface Device
bInterfaceSubClass 0 No Subclass
bInterfaceProtocol 0 None
iInterface 0
HID Device Descriptor:
bLength 9
bDescriptorType 33
bcdHID 1.11
bCountryCode 0 Not supported
bNumDescriptors 1
bDescriptorType 34 Report
wDescriptorLength 25
Report Descriptors:
** UNAVAILABLE **
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82 EP 2 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x0004 1x 4 bytes
bInterval 1
Interface Descriptor:
bLength 9
bDescriptorType 4
bInterfaceNumber 2
bAlternateSetting 0
bNumEndpoints 1
bInterfaceClass 3 Human Interface Device
bInterfaceSubClass 0 No Subclass
bInterfaceProtocol 0 None
iInterface 0
HID Device Descriptor:
bLength 9
bDescriptorType 33
bcdHID 1.11
bCountryCode 0 Not supported
bNumDescriptors 1
bDescriptorType 34 Report
wDescriptorLength 37
Report Descriptors:
** UNAVAILABLE **
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x83 EP 3 IN
bmAttributes 3
Transfer Type Interrupt
Synch Type None
Usage Type Data
wMaxPacketSize 0x000f 1x 15 bytes
bInterval 1
Device Status: 0x0000
(Bus Powered)
^ permalink raw reply
* Re: [PATCH v2] HID: rmi: check sanity of the incoming report
From: Jiri Kosina @ 2014-09-12 20:58 UTC (permalink / raw)
To: Andrew Duggan; +Cc: linux-input, linux-kernel, Benjamin Tissoires
In-Reply-To: <1410397358-15308-1-git-send-email-aduggan@synaptics.com>
On Wed, 10 Sep 2014, Andrew Duggan wrote:
> From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> In the Dell XPS 13 9333, it appears that sometimes the bus get confused
> and corrupts the incoming data. It fills the input report with the
> sentinel value "ff". Synaptics told us that such behavior does not comes
> from the touchpad itself, so we filter out such reports here.
>
> Unfortunately, we can not simply discard the incoming data because they
> may contain useful information. Most of the time, the misbehavior is
> quite near the end of the report, so we can still use the valid part of
> it.
>
> Fixes:
> https://bugzilla.redhat.com/show_bug.cgi?id=1123584
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 3/3] psmouse: Add support for detecting FocalTech PS/2 touchpads
From: Dmitry Torokhov @ 2014-09-13 0:26 UTC (permalink / raw)
To: Hans de Goede; +Cc: Peter Hutterer, Benjamin Tissoires, linux-input
In-Reply-To: <1410512912-11609-4-git-send-email-hdegoede@redhat.com>
Hi Hans,
On Fri, Sep 12, 2014 at 11:08:32AM +0200, Hans de Goede wrote:
> +/* Always check for focaltech, this is safe as it uses pnp-id matching */
> + if (psmouse_do_detect(focaltech_detect, psmouse, set_properties) == 0) {
> + if (!set_properties || focaltech_init(psmouse) == 0) {
> + /* Not supported yet, use bare protocol */
> + psmouse_max_proto = PSMOUSE_PS2;
I do not believe we need to muck with psmouse_max_proto here, so I'll
drop it and apply.
Thanks.
> + return PSMOUSE_PS2;
> + }
> + }
> +
> /*
> * We always check for lifebook because it does not disturb mouse
> * (it only checks DMI information).
> --
> 2.1.0
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 3/3] psmouse: Add support for detecting FocalTech PS/2 touchpads
From: Hans de Goede @ 2014-09-13 8:10 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Peter Hutterer, Benjamin Tissoires, linux-input
In-Reply-To: <20140913002629.GA12622@core.coreip.homeip.net>
Hi,
On 09/13/2014 02:26 AM, Dmitry Torokhov wrote:
> Hi Hans,
>
> On Fri, Sep 12, 2014 at 11:08:32AM +0200, Hans de Goede wrote:
>> +/* Always check for focaltech, this is safe as it uses pnp-id matching */
>> + if (psmouse_do_detect(focaltech_detect, psmouse, set_properties) == 0) {
>> + if (!set_properties || focaltech_init(psmouse) == 0) {
>> + /* Not supported yet, use bare protocol */
>> + psmouse_max_proto = PSMOUSE_PS2;
>
> I do not believe we need to muck with psmouse_max_proto here, so I'll
> drop it and apply.
Oh, but we do need to set psmouse_max_proto, otherwise this won't work, as
I already tried to explain in the review of v1 (but clearly failed to do so).
psmouse_initialize() checks psmouse_max_proto, and if it is not set to
PSMOUSE_PS2 does things which upsets these touchpads.
Regards,
Hans
^ permalink raw reply
* [PATCH 1/2] hid: sony: Corrections for the DualShock 4 HID descriptor
From: Frank Praznik @ 2014-09-14 15:56 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, Frank Praznik
Fix a few minor issues in the HID descriptor:
- A 6 bit entry had a logical maximum of 255 when the largest it can be is 63.
- A logical max value was incorrectly being set to -1 instead of 255.
- Set the min/max of the gyroscopes to -8192/8191 as that is the range of
values which represent the true controller orientation. Any values beyond
those extents are just noise.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
drivers/hid/hid-sony.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index ecc6616..75da56d 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -176,7 +176,7 @@ static u8 dualshock4_usb_rdesc[] = {
0x75, 0x06, /* Report Size (6), */
0x95, 0x01, /* Report Count (1), */
0x15, 0x00, /* Logical Minimum (0), */
- 0x25, 0x7F, /* Logical Maximum (127), */
+ 0x25, 0x3F, /* Logical Maximum (63), */
0x81, 0x02, /* Input (Variable), */
0x05, 0x01, /* Usage Page (Desktop), */
0x09, 0x33, /* Usage (Rx), */
@@ -200,14 +200,14 @@ static u8 dualshock4_usb_rdesc[] = {
0x81, 0x02, /* Input (Variable), */
0x19, 0x43, /* Usage Minimum (43h), */
0x29, 0x45, /* Usage Maximum (45h), */
- 0x16, 0xFF, 0xBF, /* Logical Minimum (-16385), */
- 0x26, 0x00, 0x40, /* Logical Maximum (16384), */
+ 0x16, 0x00, 0xE0, /* Logical Minimum (-8192), */
+ 0x26, 0xFF, 0x1F, /* Logical Maximum (8191), */
0x95, 0x03, /* Report Count (3), */
0x81, 0x02, /* Input (Variable), */
0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
0x09, 0x21, /* Usage (21h), */
0x15, 0x00, /* Logical Minimum (0), */
- 0x25, 0xFF, /* Logical Maximum (255), */
+ 0x26, 0xFF, 0x00, /* Logical Maximum (255), */
0x75, 0x08, /* Report Size (8), */
0x95, 0x27, /* Report Count (39), */
0x81, 0x02, /* Input (Variable), */
@@ -509,8 +509,8 @@ static u8 dualshock4_bt_rdesc[] = {
0x81, 0x02, /* Input (Variable), */
0x19, 0x43, /* Usage Minimum (43h), */
0x29, 0x45, /* Usage Maximum (45h), */
- 0x16, 0xFF, 0xBF, /* Logical Minimum (-16385), */
- 0x26, 0x00, 0x40, /* Logical Maximum (16384), */
+ 0x16, 0x00, 0xE0, /* Logical Minimum (-8192), */
+ 0x26, 0xFF, 0x1F, /* Logical Maximum (8191), */
0x95, 0x03, /* Report Count (3), */
0x81, 0x02, /* Input (Variable), */
0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
--
2.1.0
^ permalink raw reply related
* [PATCH 2/2] hid: sony: Update file header and correct comments
From: Frank Praznik @ 2014-09-14 15:56 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, Frank Praznik
In-Reply-To: <1410710199-14383-1-git-send-email-frank.praznik@oh.rr.com>
Update the file header and correct an outdated comment block.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
drivers/hid/hid-sony.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 75da56d..96a8ec5 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1,5 +1,5 @@
/*
- * HID driver for Sony / PS2 / PS3 BD devices.
+ * HID driver for Sony / PS2 / PS3 / PS4 BD devices.
*
* Copyright (c) 1999 Andreas Gal
* Copyright (c) 2000-2005 Vojtech Pavlik <vojtech@suse.cz>
@@ -8,6 +8,7 @@
* Copyright (c) 2012 David Dillow <dave@thedillows.org>
* Copyright (c) 2006-2013 Jiri Kosina
* Copyright (c) 2013 Colin Leitner <colin.leitner@gmail.com>
+ * Copyright (c) 2014 Frank Praznik <frank.praznik@gmail.com>
*/
/*
@@ -395,11 +396,11 @@ static u8 dualshock4_usb_rdesc[] = {
/*
* The default behavior of the Dualshock 4 is to send reports using report
- * type 1 when running over Bluetooth. However, as soon as it receives a
- * report of type 17 to set the LEDs or rumble it starts returning it's state
- * in report 17 instead of 1. Since report 17 is undefined in the default HID
+ * type 1 when running over Bluetooth. However, when feature report 2 is
+ * requested during the controller initialization it starts sending input
+ * reports in report 17. Since report 17 is undefined in the default HID
* descriptor the button and axis definitions must be moved to report 17 or
- * the HID layer won't process the received input once a report is sent.
+ * the HID layer won't process the received input.
*/
static u8 dualshock4_bt_rdesc[] = {
0x05, 0x01, /* Usage Page (Desktop), */
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] Handle spurious backslash key repeats on some keyboards
From: David Herrmann @ 2014-09-14 16:45 UTC (permalink / raw)
To: Fredrik Hallenberg, Jiri Kosina, Dmitry Torokhov; +Cc: open list:HID CORE LAYER
In-Reply-To: <1407664566-5303-1-git-send-email-megahallon@gmail.com>
Hi
On Sun, Aug 10, 2014 at 11:56 AM, Fredrik Hallenberg
<megahallon@gmail.com> wrote:
> Here is my attempt on a fix for bug 70181, please review it. It is
> tested on my nordic Corsair K70, if this solution is deemed acceptable
> I can ask some people with other problematic keyboards to test it.
>
> Signed-off-by: Fredrik Hallenberg <megahallon@gmail.com>
> ---
> drivers/hid/hid-input.c | 14 ++++++++++++++
> include/linux/hid.h | 1 +
> 2 files changed, 15 insertions(+)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 2619f7f..56429c0 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -1085,6 +1085,20 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
> return;
> }
>
> + /*
> + * Some keyboards will report both HID keys 0x31 (\ and |) and
> + * 0x32 (Non-US # and ~) which are both mapped to
> + * KEY_BACKSLASH. This will cause spurious events causing
> + * repeated backslash key presses. Handle this by tracking the
> + * active HID code and ignoring the other one.
> + */
> + if (usage->type == EV_KEY && usage->code == KEY_BACKSLASH) {
> + if (value)
> + field->hidinput->backslash_usage = usage->hid;
> + else if (field->hidinput->backslash_usage != usage->hid)
> + return;
> + }
> +
Ok, I have looked through some more reports and at a bunch of AT
keyboards and how the HID standard maps them. The backslash/pipe key
is the only key affected by this. There _might_ be some similar
overlap with Korean 106 and Japanese 109 layouts, but as there haven't
been any reports, we probably don't care right now.
And indeed, I'm kinda reluctant to add an hwdb entry for the reported
keyboards as I couldn't find anyone with non-105 keyboards to compare
VID/PID. Furthermore, similar issues will probably show up again in
the future. However, I still dislike the quick hack in this patch. I
especially dislike matching on KEY_BACKSLASH while we really want to
match on 0x31 and 0x32. If users use setkeycode() to change a mapping,
this should not trigger our quirk. We could easily change the patch to
match on usage, but I think there's a better way:
hid-core.c: hid_input_field()
This is the main entry point for data that is matched on HID fields.
We already have a RollOver quirk in there, which is extremely bad
documented and I have no idea what it does.. However, this function
saves the reported values so we can retrieve them on a following
report. Why don't we skip events that have the same value reported as
before? Obviously, we should do this only for absolute data, not
relative. So I think something like this should work (sorry for
line-wraps):
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 12b6e67..000a768 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1220,7 +1220,10 @@
for (n = 0; n < count; n++) {
if (HID_MAIN_ITEM_VARIABLE & field->flags) {
- hid_process_event(hid, field,
&field->usage[n], value[n], interrupt);
+ /* ignore absolute values if they didn't change */
+ if (HID_MAIN_ITEM_RELATIVE & field->flags ||
+ value[n] != field->value[n])
+ hid_process_event(hid, field,
&field->usage[n], value[n], interrupt);
continue;
}
@Jiri: Any comments on this? I now agree with Fredrik that this is
better solved in HID core. It is a generic problem..
In HID, we save values per scancode / HID-field, additionally to
input-core saving them per keycode / input-code. This patch basically
makes use of this and avoids sending events for scan-codes that didn't
change, but accidentally map to the same keycode as another key (which
might have a different state than this one).
If this looks reasonable, I can prepare a patch with a proper
commit-log and give it a spin here.
Thanks
David
> /* report the usage code as scancode if the key status has changed */
> if (usage->type == EV_KEY && !!test_bit(usage->code, input->key) != value)
> input_event(input, EV_MSC, MSC_SCAN, usage->hid);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index f53c4a9..1c59f8f 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -448,6 +448,7 @@ struct hid_input {
> struct list_head list;
> struct hid_report *report;
> struct input_dev *input;
> + unsigned backslash_usage;
> };
>
> enum hid_type {
> --
> 2.1.0.rc1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] Handle spurious backslash key repeats on some keyboards
From: David Herrmann @ 2014-09-14 17:03 UTC (permalink / raw)
To: Fredrik Hallenberg, Jiri Kosina, Dmitry Torokhov; +Cc: open list:HID CORE LAYER
In-Reply-To: <CANq1E4RVV1nEbtiAkiePtSF-K8CdnS7hmts_8hqDOi=mWsE4MA@mail.gmail.com>
Hi
On Sun, Sep 14, 2014 at 6:45 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Sun, Aug 10, 2014 at 11:56 AM, Fredrik Hallenberg
> <megahallon@gmail.com> wrote:
>> Here is my attempt on a fix for bug 70181, please review it. It is
>> tested on my nordic Corsair K70, if this solution is deemed acceptable
>> I can ask some people with other problematic keyboards to test it.
>>
>> Signed-off-by: Fredrik Hallenberg <megahallon@gmail.com>
>> ---
>> drivers/hid/hid-input.c | 14 ++++++++++++++
>> include/linux/hid.h | 1 +
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
>> index 2619f7f..56429c0 100644
>> --- a/drivers/hid/hid-input.c
>> +++ b/drivers/hid/hid-input.c
>> @@ -1085,6 +1085,20 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
>> return;
>> }
>>
>> + /*
>> + * Some keyboards will report both HID keys 0x31 (\ and |) and
>> + * 0x32 (Non-US # and ~) which are both mapped to
>> + * KEY_BACKSLASH. This will cause spurious events causing
>> + * repeated backslash key presses. Handle this by tracking the
>> + * active HID code and ignoring the other one.
>> + */
>> + if (usage->type == EV_KEY && usage->code == KEY_BACKSLASH) {
>> + if (value)
>> + field->hidinput->backslash_usage = usage->hid;
>> + else if (field->hidinput->backslash_usage != usage->hid)
>> + return;
>> + }
>> +
>
> Ok, I have looked through some more reports and at a bunch of AT
> keyboards and how the HID standard maps them. The backslash/pipe key
> is the only key affected by this. There _might_ be some similar
> overlap with Korean 106 and Japanese 109 layouts, but as there haven't
> been any reports, we probably don't care right now.
>
> And indeed, I'm kinda reluctant to add an hwdb entry for the reported
> keyboards as I couldn't find anyone with non-105 keyboards to compare
> VID/PID. Furthermore, similar issues will probably show up again in
> the future. However, I still dislike the quick hack in this patch. I
> especially dislike matching on KEY_BACKSLASH while we really want to
> match on 0x31 and 0x32. If users use setkeycode() to change a mapping,
> this should not trigger our quirk. We could easily change the patch to
> match on usage, but I think there's a better way:
>
> hid-core.c: hid_input_field()
> This is the main entry point for data that is matched on HID fields.
> We already have a RollOver quirk in there, which is extremely bad
> documented and I have no idea what it does.. However, this function
> saves the reported values so we can retrieve them on a following
> report. Why don't we skip events that have the same value reported as
> before? Obviously, we should do this only for absolute data, not
> relative. So I think something like this should work (sorry for
> line-wraps):
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 12b6e67..000a768 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1220,7 +1220,10 @@
> for (n = 0; n < count; n++) {
>
> if (HID_MAIN_ITEM_VARIABLE & field->flags) {
> - hid_process_event(hid, field,
> &field->usage[n], value[n], interrupt);
> + /* ignore absolute values if they didn't change */
> + if (HID_MAIN_ITEM_RELATIVE & field->flags ||
> + value[n] != field->value[n])
> + hid_process_event(hid, field,
> &field->usage[n], value[n], interrupt);
This should probably be:
(HID_MAIN_ITEM_RELATIVE | HID_MAIN_ITEM_BUFFERED_BYTE)
..though I wonder whether buffered-byte makes much sense without "relative"..
^ permalink raw reply
* Re: [PATCH] Handle spurious backslash key repeats on some keyboards
From: Fredrik Hallenberg @ 2014-09-14 22:53 UTC (permalink / raw)
To: David Herrmann; +Cc: Jiri Kosina, Dmitry Torokhov, open list:HID CORE LAYER
In-Reply-To: <CANq1E4RVV1nEbtiAkiePtSF-K8CdnS7hmts_8hqDOi=mWsE4MA@mail.gmail.com>
Thank you for this, I felt that it would be difficult to do a good
solution with hwdb but could not really back this up. It is also nice
to see a much better patch than mine, I did not have enough knowledge
on the input system to do a proper one.
On Sun, Sep 14, 2014 at 6:45 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Sun, Aug 10, 2014 at 11:56 AM, Fredrik Hallenberg
> <megahallon@gmail.com> wrote:
>> Here is my attempt on a fix for bug 70181, please review it. It is
>> tested on my nordic Corsair K70, if this solution is deemed acceptable
>> I can ask some people with other problematic keyboards to test it.
>>
>> Signed-off-by: Fredrik Hallenberg <megahallon@gmail.com>
>> ---
>> drivers/hid/hid-input.c | 14 ++++++++++++++
>> include/linux/hid.h | 1 +
>> 2 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
>> index 2619f7f..56429c0 100644
>> --- a/drivers/hid/hid-input.c
>> +++ b/drivers/hid/hid-input.c
>> @@ -1085,6 +1085,20 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
>> return;
>> }
>>
>> + /*
>> + * Some keyboards will report both HID keys 0x31 (\ and |) and
>> + * 0x32 (Non-US # and ~) which are both mapped to
>> + * KEY_BACKSLASH. This will cause spurious events causing
>> + * repeated backslash key presses. Handle this by tracking the
>> + * active HID code and ignoring the other one.
>> + */
>> + if (usage->type == EV_KEY && usage->code == KEY_BACKSLASH) {
>> + if (value)
>> + field->hidinput->backslash_usage = usage->hid;
>> + else if (field->hidinput->backslash_usage != usage->hid)
>> + return;
>> + }
>> +
>
> Ok, I have looked through some more reports and at a bunch of AT
> keyboards and how the HID standard maps them. The backslash/pipe key
> is the only key affected by this. There _might_ be some similar
> overlap with Korean 106 and Japanese 109 layouts, but as there haven't
> been any reports, we probably don't care right now.
>
> And indeed, I'm kinda reluctant to add an hwdb entry for the reported
> keyboards as I couldn't find anyone with non-105 keyboards to compare
> VID/PID. Furthermore, similar issues will probably show up again in
> the future. However, I still dislike the quick hack in this patch. I
> especially dislike matching on KEY_BACKSLASH while we really want to
> match on 0x31 and 0x32. If users use setkeycode() to change a mapping,
> this should not trigger our quirk. We could easily change the patch to
> match on usage, but I think there's a better way:
>
> hid-core.c: hid_input_field()
> This is the main entry point for data that is matched on HID fields.
> We already have a RollOver quirk in there, which is extremely bad
> documented and I have no idea what it does.. However, this function
> saves the reported values so we can retrieve them on a following
> report. Why don't we skip events that have the same value reported as
> before? Obviously, we should do this only for absolute data, not
> relative. So I think something like this should work (sorry for
> line-wraps):
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 12b6e67..000a768 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1220,7 +1220,10 @@
> for (n = 0; n < count; n++) {
>
> if (HID_MAIN_ITEM_VARIABLE & field->flags) {
> - hid_process_event(hid, field,
> &field->usage[n], value[n], interrupt);
> + /* ignore absolute values if they didn't change */
> + if (HID_MAIN_ITEM_RELATIVE & field->flags ||
> + value[n] != field->value[n])
> + hid_process_event(hid, field,
> &field->usage[n], value[n], interrupt);
> continue;
> }
>
>
> @Jiri: Any comments on this? I now agree with Fredrik that this is
> better solved in HID core. It is a generic problem..
>
> In HID, we save values per scancode / HID-field, additionally to
> input-core saving them per keycode / input-code. This patch basically
> makes use of this and avoids sending events for scan-codes that didn't
> change, but accidentally map to the same keycode as another key (which
> might have a different state than this one).
>
> If this looks reasonable, I can prepare a patch with a proper
> commit-log and give it a spin here.
>
> Thanks
> David
>
>> /* report the usage code as scancode if the key status has changed */
>> if (usage->type == EV_KEY && !!test_bit(usage->code, input->key) != value)
>> input_event(input, EV_MSC, MSC_SCAN, usage->hid);
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index f53c4a9..1c59f8f 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -448,6 +448,7 @@ struct hid_input {
>> struct list_head list;
>> struct hid_report *report;
>> struct input_dev *input;
>> + unsigned backslash_usage;
>> };
>>
>> enum hid_type {
>> --
>> 2.1.0.rc1
>>
>> --
>> 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] HID: add new gamepad LED constants
From: Michael Wright @ 2014-09-15 3:13 UTC (permalink / raw)
To: linux-input; +Cc: Michael Wright
Improve gamepad support by introducing new LED constants for player
LEDs and the mappings from their corresponding HID usages introduced
in HUTTR47: http://www.usb.org/developers/hidpage/HUTRR47.pdf
Change-Id: If25d8a8e2570dfab3a35f9be5b1d03ab662f6b1c
Signed-off-by: Michael Wright <michaelwr@google.com>
---
drivers/hid/hid-input.c | 4 ++++
include/uapi/linux/input.h | 4 ++++
2 files changed, 8 insertions(+)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 2619f7f..a1cb3bf 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -633,6 +633,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
case 0x4b: map_led (LED_MISC); break; /* "Generic Indicator" */
case 0x19: map_led (LED_MAIL); break; /* "Message Waiting" */
case 0x4d: map_led (LED_CHARGING); break; /* "External Power Connected" */
+ case 0x4f: map_led (LED_PLAYER_1); break; /* "Player 1" */
+ case 0x50: map_led (LED_PLAYER_2); break; /* "Player 2" */
+ case 0x51: map_led (LED_PLAYER_3); break; /* "Player 3" */
+ case 0x52: map_led (LED_PLAYER_4); break; /* "Player 4" */
default: goto ignore;
}
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 1874ebe..bb43f20 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -908,6 +908,10 @@ struct input_keymap_entry {
#define LED_MISC 0x08
#define LED_MAIL 0x09
#define LED_CHARGING 0x0a
+#define LED_PLAYER_1 0x0b
+#define LED_PLAYER_2 0x0c
+#define LED_PLAYER_3 0x0d
+#define LED_PLAYER_4 0x0e
#define LED_MAX 0x0f
#define LED_CNT (LED_MAX+1)
--
2.1.0.rc2.206.gedb03e5
^ permalink raw reply related
* Re: [PATCH] HID: add new gamepad LED constants
From: David Herrmann @ 2014-09-15 5:07 UTC (permalink / raw)
To: Michael Wright
Cc: open list:HID CORE LAYER, Michael Wright, Dmitry Torokhov,
Jiri Kosina
In-Reply-To: <1410750811-11156-1-git-send-email-michaelwr@google.com>
Hi
On Mon, Sep 15, 2014 at 5:13 AM, Michael Wright <michaelwr@android.com> wrote:
> Improve gamepad support by introducing new LED constants for player
> LEDs and the mappings from their corresponding HID usages introduced
> in HUTTR47: http://www.usb.org/developers/hidpage/HUTRR47.pdf
Why not introduce all 8 player LEDs as described in the document? You
should be safe increasing LED_MAX to 0x1f.
Otherwise, looks good to me. But please don't forgot to put maintainers on CC.
Thanks
David
> Change-Id: If25d8a8e2570dfab3a35f9be5b1d03ab662f6b1c
> Signed-off-by: Michael Wright <michaelwr@google.com>
> ---
> drivers/hid/hid-input.c | 4 ++++
> include/uapi/linux/input.h | 4 ++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 2619f7f..a1cb3bf 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -633,6 +633,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
> case 0x4b: map_led (LED_MISC); break; /* "Generic Indicator" */
> case 0x19: map_led (LED_MAIL); break; /* "Message Waiting" */
> case 0x4d: map_led (LED_CHARGING); break; /* "External Power Connected" */
> + case 0x4f: map_led (LED_PLAYER_1); break; /* "Player 1" */
> + case 0x50: map_led (LED_PLAYER_2); break; /* "Player 2" */
> + case 0x51: map_led (LED_PLAYER_3); break; /* "Player 3" */
> + case 0x52: map_led (LED_PLAYER_4); break; /* "Player 4" */
>
> default: goto ignore;
> }
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index 1874ebe..bb43f20 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -908,6 +908,10 @@ struct input_keymap_entry {
> #define LED_MISC 0x08
> #define LED_MAIL 0x09
> #define LED_CHARGING 0x0a
> +#define LED_PLAYER_1 0x0b
> +#define LED_PLAYER_2 0x0c
> +#define LED_PLAYER_3 0x0d
> +#define LED_PLAYER_4 0x0e
> #define LED_MAX 0x0f
> #define LED_CNT (LED_MAX+1)
>
> --
> 2.1.0.rc2.206.gedb03e5
>
> --
> 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] Handle spurious backslash key repeats on some keyboards
From: Fredrik Hallenberg @ 2014-09-15 8:29 UTC (permalink / raw)
To: David Herrmann; +Cc: Jiri Kosina, Dmitry Torokhov, open list:HID CORE LAYER
In-Reply-To: <CAMsZVf9k814m+OH=B_gXnnM64+soc_cV9ztm87FoHpfCZyDAMQ@mail.gmail.com>
FYI I just got a "lsusb -v" dump from a US version of my keyboard, and
it is exactly the same as the one from my nordic version.
On Mon, Sep 15, 2014 at 12:53 AM, Fredrik Hallenberg
<megahallon@gmail.com> wrote:
> Thank you for this, I felt that it would be difficult to do a good
> solution with hwdb but could not really back this up. It is also nice
> to see a much better patch than mine, I did not have enough knowledge
> on the input system to do a proper one.
>
> On Sun, Sep 14, 2014 at 6:45 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> Hi
>>
>> On Sun, Aug 10, 2014 at 11:56 AM, Fredrik Hallenberg
>> <megahallon@gmail.com> wrote:
>>> Here is my attempt on a fix for bug 70181, please review it. It is
>>> tested on my nordic Corsair K70, if this solution is deemed acceptable
>>> I can ask some people with other problematic keyboards to test it.
>>>
>>> Signed-off-by: Fredrik Hallenberg <megahallon@gmail.com>
>>> ---
>>> drivers/hid/hid-input.c | 14 ++++++++++++++
>>> include/linux/hid.h | 1 +
>>> 2 files changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
>>> index 2619f7f..56429c0 100644
>>> --- a/drivers/hid/hid-input.c
>>> +++ b/drivers/hid/hid-input.c
>>> @@ -1085,6 +1085,20 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
>>> return;
>>> }
>>>
>>> + /*
>>> + * Some keyboards will report both HID keys 0x31 (\ and |) and
>>> + * 0x32 (Non-US # and ~) which are both mapped to
>>> + * KEY_BACKSLASH. This will cause spurious events causing
>>> + * repeated backslash key presses. Handle this by tracking the
>>> + * active HID code and ignoring the other one.
>>> + */
>>> + if (usage->type == EV_KEY && usage->code == KEY_BACKSLASH) {
>>> + if (value)
>>> + field->hidinput->backslash_usage = usage->hid;
>>> + else if (field->hidinput->backslash_usage != usage->hid)
>>> + return;
>>> + }
>>> +
>>
>> Ok, I have looked through some more reports and at a bunch of AT
>> keyboards and how the HID standard maps them. The backslash/pipe key
>> is the only key affected by this. There _might_ be some similar
>> overlap with Korean 106 and Japanese 109 layouts, but as there haven't
>> been any reports, we probably don't care right now.
>>
>> And indeed, I'm kinda reluctant to add an hwdb entry for the reported
>> keyboards as I couldn't find anyone with non-105 keyboards to compare
>> VID/PID. Furthermore, similar issues will probably show up again in
>> the future. However, I still dislike the quick hack in this patch. I
>> especially dislike matching on KEY_BACKSLASH while we really want to
>> match on 0x31 and 0x32. If users use setkeycode() to change a mapping,
>> this should not trigger our quirk. We could easily change the patch to
>> match on usage, but I think there's a better way:
>>
>> hid-core.c: hid_input_field()
>> This is the main entry point for data that is matched on HID fields.
>> We already have a RollOver quirk in there, which is extremely bad
>> documented and I have no idea what it does.. However, this function
>> saves the reported values so we can retrieve them on a following
>> report. Why don't we skip events that have the same value reported as
>> before? Obviously, we should do this only for absolute data, not
>> relative. So I think something like this should work (sorry for
>> line-wraps):
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 12b6e67..000a768 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -1220,7 +1220,10 @@
>> for (n = 0; n < count; n++) {
>>
>> if (HID_MAIN_ITEM_VARIABLE & field->flags) {
>> - hid_process_event(hid, field,
>> &field->usage[n], value[n], interrupt);
>> + /* ignore absolute values if they didn't change */
>> + if (HID_MAIN_ITEM_RELATIVE & field->flags ||
>> + value[n] != field->value[n])
>> + hid_process_event(hid, field,
>> &field->usage[n], value[n], interrupt);
>> continue;
>> }
>>
>>
>> @Jiri: Any comments on this? I now agree with Fredrik that this is
>> better solved in HID core. It is a generic problem..
>>
>> In HID, we save values per scancode / HID-field, additionally to
>> input-core saving them per keycode / input-code. This patch basically
>> makes use of this and avoids sending events for scan-codes that didn't
>> change, but accidentally map to the same keycode as another key (which
>> might have a different state than this one).
>>
>> If this looks reasonable, I can prepare a patch with a proper
>> commit-log and give it a spin here.
>>
>> Thanks
>> David
>>
>>> /* report the usage code as scancode if the key status has changed */
>>> if (usage->type == EV_KEY && !!test_bit(usage->code, input->key) != value)
>>> input_event(input, EV_MSC, MSC_SCAN, usage->hid);
>>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>>> index f53c4a9..1c59f8f 100644
>>> --- a/include/linux/hid.h
>>> +++ b/include/linux/hid.h
>>> @@ -448,6 +448,7 @@ struct hid_input {
>>> struct list_head list;
>>> struct hid_report *report;
>>> struct input_dev *input;
>>> + unsigned backslash_usage;
>>> };
>>>
>>> enum hid_type {
>>> --
>>> 2.1.0.rc1
>>>
>>> --
>>> 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
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