* Re: [PATCH] mc13xxx-ts: use zero as default value if no pdata was defined
From: Michael Grzeschik @ 2013-08-14 3:36 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Michael Grzeschik, linux-input, linux-kernel, kernel,
Samuel Ortiz
In-Reply-To: <20130813164609.GA21548@core.coreip.homeip.net>
Hi Dimitry,
On Tue, Aug 13, 2013 at 09:46:09AM -0700, Dmitry Torokhov wrote:
> Hi Michael,
>
> On Tue, Aug 13, 2013 at 02:14:30PM +0200, Michael Grzeschik wrote:
> > In case of devicetree, we currently don't have a way to append pdata for
> > the touchscreen. The current approach is to bail out in that case.
> > This patch makes it possible to probe the touchscreen without pdata
> > and use zero as default values for the atox and ato adc conversion.
>
> I'd rather you added the devicetree support to the driver.
I know that we will need real devictree glue that generates pdata in the
long run. I am working on that. Beside that, for now this patch makes
sense anyway.
Regards,
Michael
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH 1/1] Input: htcpen - Fix incorrect placement of __initdata
From: Sachin Kamat @ 2013-08-14 6:25 UTC (permalink / raw)
To: linux-input@vger.kernel.org; +Cc: Dmitry Torokhov, Sachin Kamat, Pau Oliva Fora
In-Reply-To: <1375868346-28851-1-git-send-email-sachin.kamat@linaro.org>
Hi Dmitry,
On 7 August 2013 15:09, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> __initdata should be placed between the variable name and equal
> sign for the variable to be placed in the intended section.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> Cc: Pau Oliva Fora <pau@eslack.org>
> ---
> drivers/input/touchscreen/htcpen.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/htcpen.c b/drivers/input/touchscreen/htcpen.c
> index 6c4fb84..6650085 100644
> --- a/drivers/input/touchscreen/htcpen.c
> +++ b/drivers/input/touchscreen/htcpen.c
> @@ -221,7 +221,7 @@ static struct isa_driver htcpen_isa_driver = {
> }
> };
>
> -static struct dmi_system_id __initdata htcshift_dmi_table[] = {
> +static struct dmi_system_id htcshift_dmi_table[] __initdata = {
> {
> .ident = "Shift",
> .matches = {
> --
> 1.7.9.5
>
Thanks for applying my other patches. Looks like you missed this one though :)
--
With warm regards,
Sachin
^ permalink raw reply
* Re: [PATCH 1/3] HID: Use existing parser for pre-scanning the report descriptors
From: Alexander Holler @ 2013-08-14 6:46 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Stephane Chatty,
Mika Westerberg, linux-input, linux-kernel@vger.kernel.org,
Srinivas Pandruvada
In-Reply-To: <CAN+gG=EkGToAGatxqywBatJOjcWUF6=u1SZ86vVmUPnujJMJvw@mail.gmail.com>
Am 13.08.2013 21:15, schrieb Benjamin Tissoires:
> On Tue, Aug 13, 2013 at 8:37 PM, Alexander Holler <holler@ahsoftware.de> wrote:
>>
>> Another problem is that I don't have any commercial sensor hub and I'm
>> therefor not a very relvant as tester (I've implemented the firmware for my
>> HID (sensor hub) device too). Therefor I've added Srinivas Pandruvada to cc,
>> because he's the only one I know who has HID sensor hubs.
>
> It is not a problem if you don't have a commercial sensor hub :)
> Thanks for adding Srinivas in CC.
The problem is that this invalidates me as a tester because everything I
can test is something untested. As I've written both ends of the pipe, I
could have understood something totally wrong but as I've implemented
both sides (the device- and the kernel-part) it still might work without
problems. Therefor I currently have to bother Srinivas whenever I do
changes in the kernel in regard to hid-sensor-hub. ;)
>>
>> And, as said, I've implemented the other side here too, therefor I've added
>> the descriptor I'm using below.
>>
>
> Thanks, this will help me to test against your report descriptors.
> Can I also ask you to send me some hid-recorder[1] traces of your
> sensor? With hid-replay, I can then re-inject them in the hid
> subsystem, and then I include the results in a regression test suite.
Unfortunately hid-record doesn't list my device because it doesn't show
up as an hidraw-device.
But maybe you have an Arduino Leonardo or a clone. If so you can get the
description and firmware here:
http://ahsoftware.de/dcf77-hid-usb-rtc/
Regards,
Alexander Holler
^ permalink raw reply
* Re: [PATCH] input: qt1070: add power management ops
From: Nicolas Ferre @ 2013-08-14 6:52 UTC (permalink / raw)
To: Bo Shen; +Cc: dmitry.torokhov, Wenyou.Yang, linux-arm-kernel, linux-input
In-Reply-To: <1376379826-28808-1-git-send-email-voice.shen@atmel.com>
On 13/08/2013 09:43, Bo Shen :
> Add power management ops for qt1070, it maybe a wakeup source
>
> Signed-off-by: Bo Shen <voice.shen@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> drivers/input/keyboard/qt1070.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/input/keyboard/qt1070.c b/drivers/input/keyboard/qt1070.c
> index 42b773b..deefe5a 100644
> --- a/drivers/input/keyboard/qt1070.c
> +++ b/drivers/input/keyboard/qt1070.c
> @@ -243,6 +243,30 @@ static int qt1070_remove(struct i2c_client *client)
> return 0;
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +static int qt1070_suspend(struct device *dev)
> +{
> + struct qt1070_data *data = dev_get_drvdata(dev);
> +
> + if (device_may_wakeup(dev))
> + enable_irq_wake(data->irq);
> +
> + return 0;
> +}
> +
> +static int qt1070_resume(struct device *dev)
> +{
> + struct qt1070_data *data = dev_get_drvdata(dev);
> +
> + if (device_may_wakeup(dev))
> + disable_irq_wake(data->irq);
> +
> + return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(qt1070_pm_ops, qt1070_suspend, qt1070_resume);
> +
> static const struct i2c_device_id qt1070_id[] = {
> { "qt1070", 0 },
> { },
> @@ -253,6 +277,7 @@ static struct i2c_driver qt1070_driver = {
> .driver = {
> .name = "qt1070",
> .owner = THIS_MODULE,
> + .pm = &qt1070_pm_ops,
> },
> .id_table = qt1070_id,
> .probe = qt1070_probe,
>
--
Nicolas Ferre
^ permalink raw reply
* [PATCH 1/4] hid-sensor-hub: fix style of comments
From: Andy Shevchenko @ 2013-08-14 8:07 UTC (permalink / raw)
To: Jiri Kosina, linux-input, Srinivas Pandruvada; +Cc: Andy Shevchenko
This patch fixes the style of the comments to be like following
/* The commentary */
There is no functional change.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-sensor-hub.c | 2 +-
include/linux/hid-sensor-hub.h | 2 +-
include/linux/hid-sensor-ids.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index ca749810..ffc80cf 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -416,7 +416,7 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
return 1;
ptr = raw_data;
- ptr++; /*Skip report id*/
+ ptr++; /* Skip report id */
spin_lock_irqsave(&pdata->lock, flags);
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index ecefb73..32ba451 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -172,7 +172,7 @@ struct hid_sensor_common {
struct hid_sensor_hub_attribute_info sensitivity;
};
-/*Convert from hid unit expo to regular exponent*/
+/* Convert from hid unit expo to regular exponent */
static inline int hid_sensor_convert_exponent(int unit_expo)
{
if (unit_expo < 0x08)
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 6f24446..4f945d3 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -37,7 +37,7 @@
#define HID_USAGE_SENSOR_ANGL_VELOCITY_Y_AXIS 0x200458
#define HID_USAGE_SENSOR_ANGL_VELOCITY_Z_AXIS 0x200459
-/*ORIENTATION: Compass 3D: (200083) */
+/* ORIENTATION: Compass 3D: (200083) */
#define HID_USAGE_SENSOR_COMPASS_3D 0x200083
#define HID_USAGE_SENSOR_ORIENT_MAGN_HEADING 0x200471
#define HID_USAGE_SENSOR_ORIENT_MAGN_HEADING_X 0x200472
--
1.8.4.rc2
^ permalink raw reply related
* [PATCH 3/4] hid-sensor-hub: move to devm_kzalloc
From: Andy Shevchenko @ 2013-08-14 8:07 UTC (permalink / raw)
To: Jiri Kosina, linux-input, Srinivas Pandruvada; +Cc: Andy Shevchenko
In-Reply-To: <1376467631-20857-1-git-send-email-andriy.shevchenko@linux.intel.com>
devm_kzalloc() will manage resources freeing and allows to make error path
smaller and nicer.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-sensor-hub.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index d0687d0..4cb19df 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -477,16 +477,15 @@ static int sensor_hub_probe(struct hid_device *hdev,
struct hid_field *field;
int dev_cnt;
- sd = kzalloc(sizeof(struct sensor_hub_data), GFP_KERNEL);
+ sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
if (!sd) {
hid_err(hdev, "cannot allocate Sensor data\n");
return -ENOMEM;
}
- sd->hsdev = kzalloc(sizeof(struct hid_sensor_hub_device), GFP_KERNEL);
+ sd->hsdev = devm_kzalloc(&hdev->dev, sizeof(*sd->hsdev), GFP_KERNEL);
if (!sd->hsdev) {
hid_err(hdev, "cannot allocate hid_sensor_hub_device\n");
- ret = -ENOMEM;
- goto err_free_hub;
+ return -ENOMEM;
}
hid_set_drvdata(hdev, sd);
sd->hsdev->hdev = hdev;
@@ -498,14 +497,14 @@ static int sensor_hub_probe(struct hid_device *hdev,
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
- goto err_free;
+ return ret;
}
INIT_LIST_HEAD(&hdev->inputs);
ret = hid_hw_start(hdev, 0);
if (ret) {
hid_err(hdev, "hw start failed\n");
- goto err_free;
+ return ret;
}
ret = hid_hw_open(hdev);
if (ret) {
@@ -570,10 +569,6 @@ err_close:
hid_hw_close(hdev);
err_stop_hw:
hid_hw_stop(hdev);
-err_free:
- kfree(sd->hsdev);
-err_free_hub:
- kfree(sd);
return ret;
}
@@ -597,8 +592,6 @@ static void sensor_hub_remove(struct hid_device *hdev)
kfree(data->hid_sensor_hub_client_devs);
hid_set_drvdata(hdev, NULL);
mutex_destroy(&data->mutex);
- kfree(data->hsdev);
- kfree(data);
}
static const struct hid_device_id sensor_hub_devices[] = {
--
1.8.4.rc2
^ permalink raw reply related
* [PATCH 2/4] hid-sensor-hub: fix indentation accross the code
From: Andy Shevchenko @ 2013-08-14 8:07 UTC (permalink / raw)
To: Jiri Kosina, linux-input, Srinivas Pandruvada; +Cc: Andy Shevchenko
In-Reply-To: <1376467631-20857-1-git-send-email-andriy.shevchenko@linux.intel.com>
Patch just rearranges lines to be more compact and/or readable. Additionally it
converts double space to one in several places.
There is no functional change.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-sensor-hub.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index ffc80cf..d0687d0 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -103,8 +103,7 @@ static int sensor_hub_get_physical_device_count(
list_for_each_entry(report, &report_enum->report_list, list) {
field = report->field[0];
- if (report->maxfield && field &&
- field->physical)
+ if (report->maxfield && field && field->physical)
cnt++;
}
@@ -192,12 +191,12 @@ int sensor_hub_set_feature(struct hid_sensor_hub_device *hsdev, u32 report_id,
u32 field_index, s32 value)
{
struct hid_report *report;
- struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
+ struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
int ret = 0;
mutex_lock(&data->mutex);
report = sensor_hub_report(report_id, hsdev->hdev, HID_FEATURE_REPORT);
- if (!report || (field_index >= report->maxfield)) {
+ if (!report || (field_index >= report->maxfield)) {
ret = -EINVAL;
goto done_proc;
}
@@ -216,12 +215,12 @@ int sensor_hub_get_feature(struct hid_sensor_hub_device *hsdev, u32 report_id,
u32 field_index, s32 *value)
{
struct hid_report *report;
- struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
+ struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
int ret = 0;
mutex_lock(&data->mutex);
report = sensor_hub_report(report_id, hsdev->hdev, HID_FEATURE_REPORT);
- if (!report || (field_index >= report->maxfield)) {
+ if (!report || (field_index >= report->maxfield)) {
ret = -EINVAL;
goto done_proc;
}
@@ -241,7 +240,7 @@ int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
u32 usage_id,
u32 attr_usage_id, u32 report_id)
{
- struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
+ struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
unsigned long flags;
struct hid_report *report;
int ret_val = 0;
@@ -302,7 +301,7 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
/* Initialize with defaults */
info->usage_id = usage_id;
- info->attrib_id = attr_usage_id;
+ info->attrib_id = attr_usage_id;
info->report_id = -1;
info->index = -1;
info->units = -1;
@@ -333,7 +332,7 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
if (field->usage[j].hid ==
attr_usage_id &&
field->usage[j].collection_index ==
- collection_index) {
+ collection_index) {
sensor_hub_fill_attr_info(info,
i, report->id,
field->unit,
@@ -357,7 +356,7 @@ EXPORT_SYMBOL_GPL(sensor_hub_input_get_attribute_info);
#ifdef CONFIG_PM
static int sensor_hub_suspend(struct hid_device *hdev, pm_message_t message)
{
- struct sensor_hub_data *pdata = hid_get_drvdata(hdev);
+ struct sensor_hub_data *pdata = hid_get_drvdata(hdev);
struct hid_sensor_hub_callbacks_list *callback;
hid_dbg(hdev, " sensor_hub_suspend\n");
@@ -374,7 +373,7 @@ static int sensor_hub_suspend(struct hid_device *hdev, pm_message_t message)
static int sensor_hub_resume(struct hid_device *hdev)
{
- struct sensor_hub_data *pdata = hid_get_drvdata(hdev);
+ struct sensor_hub_data *pdata = hid_get_drvdata(hdev);
struct hid_sensor_hub_callbacks_list *callback;
hid_dbg(hdev, " sensor_hub_resume\n");
@@ -394,6 +393,7 @@ static int sensor_hub_reset_resume(struct hid_device *hdev)
return 0;
}
#endif
+
/*
* Handle raw report as sent by device
*/
@@ -421,7 +421,6 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
spin_lock_irqsave(&pdata->lock, flags);
for (i = 0; i < report->maxfield; ++i) {
-
hid_dbg(hdev, "%d collection_index:%x hid:%x sz:%x\n",
i, report->field[i]->usage->collection_index,
report->field[i]->usage->hid,
@@ -434,7 +433,7 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
pdata->pending.raw_data = kmalloc(sz, GFP_ATOMIC);
if (pdata->pending.raw_data) {
memcpy(pdata->pending.raw_data, ptr, sz);
- pdata->pending.raw_size = sz;
+ pdata->pending.raw_size = sz;
} else
pdata->pending.raw_size = 0;
complete(&pdata->pending.ready);
@@ -539,7 +538,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
field->physical) {
name = kasprintf(GFP_KERNEL, "HID-SENSOR-%x",
field->physical);
- if (name == NULL) {
+ if (name == NULL) {
hid_err(hdev, "Failed MFD device name\n");
ret = -ENOMEM;
goto err_free_names;
@@ -617,8 +616,8 @@ static struct hid_driver sensor_hub_driver = {
.raw_event = sensor_hub_raw_event,
#ifdef CONFIG_PM
.suspend = sensor_hub_suspend,
- .resume = sensor_hub_resume,
- .reset_resume = sensor_hub_reset_resume,
+ .resume = sensor_hub_resume,
+ .reset_resume = sensor_hub_reset_resume,
#endif
};
module_hid_driver(sensor_hub_driver);
--
1.8.4.rc2
^ permalink raw reply related
* [PATCH 4/4] hid-sensor-hub: change kmalloc + memcpy by kmemdup
From: Andy Shevchenko @ 2013-08-14 8:07 UTC (permalink / raw)
To: Jiri Kosina, linux-input, Srinivas Pandruvada; +Cc: Andy Shevchenko
In-Reply-To: <1376467631-20857-1-git-send-email-andriy.shevchenko@linux.intel.com>
The patch substitutes kmemdup for kmalloc followed by memcpy.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/hid/hid-sensor-hub.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 4cb19df..0794e9f 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -430,11 +430,10 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
if (pdata->pending.status && pdata->pending.attr_usage_id ==
report->field[i]->usage->hid) {
hid_dbg(hdev, "data was pending ...\n");
- pdata->pending.raw_data = kmalloc(sz, GFP_ATOMIC);
- if (pdata->pending.raw_data) {
- memcpy(pdata->pending.raw_data, ptr, sz);
+ pdata->pending.raw_data = kmemdup(ptr, sz, GFP_ATOMIC);
+ if (pdata->pending.raw_data)
pdata->pending.raw_size = sz;
- } else
+ else
pdata->pending.raw_size = 0;
complete(&pdata->pending.ready);
}
--
1.8.4.rc2
^ permalink raw reply related
* [PATCH 0/29] simplify use of devm_ioremap_resource
From: Julia Lawall @ 2013-08-14 9:11 UTC (permalink / raw)
To: dri-devel
Cc: alsa-devel, kernel-janitors, linux-fbdev, linux-ide, linux-mtd,
linux-i2c, linux-samsung-soc, linux-scsi, linux-serial,
linux-input, linux-media, linux-pwm, linux-watchdog, rtc-linux,
linux-pm, linux-gpio, linux-tegra, linux-omap, linux-arm-kernel,
linux-usb, linux-kernel, linux-spi
devm_ioremap_resource often uses the result of a call to
platform_get_resource as its last argument. devm_ioremap_resource does
appropriate error handling on this argument, so error handling can be
removed from the call site. To make the connection between the call to
platform_get_resource and the call to devm_ioremap_resource more clear, the
former is also moved down to be adjacent to the latter.
In many cases, this patch changes the specific error value that is
returned on failure of platform_get_resource.
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
@@
expression pdev,res,n,e,e1;
expression ret != 0;
identifier l;
@@
(
res = platform_get_resource(pdev, IORESOURCE_MEM, n);
- if (res == NULL) { ... \(goto l;\|return ret;\) }
e = devm_ioremap_resource(e1, res);
|
- res = platform_get_resource(pdev, IORESOURCE_MEM, n);
... when != res
- if (res == NULL) { ... \(goto l;\|return ret;\) }
... when != res
+ res = platform_get_resource(pdev, IORESOURCE_MEM, n);
e = devm_ioremap_resource(e1, res);
)
// </smpl>
^ permalink raw reply
* [PATCH 13/29] drivers/input/keyboard/tegra-kbc.c: simplify use of devm_ioremap_resource
From: Julia Lawall @ 2013-08-14 9:11 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: kernel-janitors, Stephen Warren, linux-input, linux-tegra,
linux-kernel
In-Reply-To: <1376471493-22215-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Remove unneeded error handling on the result of a call to
platform_get_resource when the value is passed to devm_ioremap_resource.
Move the call to platform_get_resource adjacent to the call to
devm_ioremap_resource to make the connection between them more clear.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression pdev,res,n,e,e1;
expression ret != 0;
identifier l;
@@
- res = platform_get_resource(pdev, IORESOURCE_MEM, n);
... when != res
- if (res == NULL) { ... \(goto l;\|return ret;\) }
... when != res
+ res = platform_get_resource(pdev, IORESOURCE_MEM, n);
e = devm_ioremap_resource(e1, res);
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/input/keyboard/tegra-kbc.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c
index b46142f..9cd20e6 100644
--- a/drivers/input/keyboard/tegra-kbc.c
+++ b/drivers/input/keyboard/tegra-kbc.c
@@ -638,12 +638,6 @@ static int tegra_kbc_probe(struct platform_device *pdev)
if (!tegra_kbc_check_pin_cfg(kbc, &num_rows))
return -EINVAL;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(&pdev->dev, "failed to get I/O memory\n");
- return -ENXIO;
- }
-
kbc->irq = platform_get_irq(pdev, 0);
if (kbc->irq < 0) {
dev_err(&pdev->dev, "failed to get keyboard IRQ\n");
@@ -658,6 +652,7 @@ static int tegra_kbc_probe(struct platform_device *pdev)
setup_timer(&kbc->timer, tegra_kbc_keypress_timer, (unsigned long)kbc);
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
kbc->mmio = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(kbc->mmio))
return PTR_ERR(kbc->mmio);
^ permalink raw reply related
* [PATCH] Input: elantech - fix packet check for v3 and v4 hardware
From: Matteo Delfino @ 2013-08-14 9:37 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
The signatures of v3 and v4 packets change depending on the value of a
hardware flag called 'crc_enabled'. The packet type detection must change
accordingly.
This patch also restores a consistency check for v4 packets inadvertently
removed by commit:
9eebed7de660c0b5ab129a9de4f89d20b60de68c
Input: elantech - fix for newer hardware versions (v7)
A note about the naming convention: v3 hardware is associated with IC body
v5 while v4 hardware is associated with IC body v6 and v7. The above commit
refers to IC body v7, not to v7 hardware.
Tested on Samsung NP730U3E (fw = 0x675f05, ICv7, crc_enabled = 1)
Tested-by: Giovanni Frigione <gio.frigione@gmail.com>
Signed-off-by: Matteo Delfino <kendatsuba@gmail.com>
---
Hello Dmitry,
this patch is based on some information provided by elantech. It has
been successfully tested on the above v4 hardware so the information
seems to be reliable. More testing is needed on v3 hardware with
crc_enabled=1 (if there's such hardware out there..). The behaviour
should be unchanged for v3 and v4 hardware with crc_enabled=0 (the
vast majority).
I was asked to CC stable kernels too since this patch restores a
consistency check on v4 packets removed by a previous commit already
queued in linux-3.5.y-queue but, considering the above, I think it's
best to leave this decision to you.
Best regards,
Matteo
drivers/input/mouse/elantech.c | 44 ++++++++++++++++++++++++++++++++++++----
drivers/input/mouse/elantech.h | 1 +
2 files changed, 41 insertions(+), 4 deletions(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 57b2637..8551dca 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -672,6 +672,7 @@ static int elantech_packet_check_v2(struct psmouse *psmouse)
*/
static int elantech_packet_check_v3(struct psmouse *psmouse)
{
+ struct elantech_data *etd = psmouse->private;
const u8 debounce_packet[] = { 0xc4, 0xff, 0xff, 0x02, 0xff, 0xff };
unsigned char *packet = psmouse->packet;
@@ -682,19 +683,48 @@ static int elantech_packet_check_v3(struct psmouse *psmouse)
if (!memcmp(packet, debounce_packet, sizeof(debounce_packet)))
return PACKET_DEBOUNCE;
- if ((packet[0] & 0x0c) == 0x04 && (packet[3] & 0xcf) == 0x02)
- return PACKET_V3_HEAD;
+ /*
+ * If the hardware flag 'crc_enabled' is set the packets have
+ * different signatures.
+ */
+ if (etd->crc_enabled) {
+ if ((packet[3] & 0x09) == 0x08)
+ return PACKET_V3_HEAD;
+
+ if ((packet[3] & 0x09) == 0x09)
+ return PACKET_V3_TAIL;
+ } else {
+ if ((packet[0] & 0x0c) == 0x04 && (packet[3] & 0xcf) == 0x02)
+ return PACKET_V3_HEAD;
- if ((packet[0] & 0x0c) == 0x0c && (packet[3] & 0xce) == 0x0c)
- return PACKET_V3_TAIL;
+ if ((packet[0] & 0x0c) == 0x0c && (packet[3] & 0xce) == 0x0c)
+ return PACKET_V3_TAIL;
+ }
return PACKET_UNKNOWN;
}
static int elantech_packet_check_v4(struct psmouse *psmouse)
{
+ struct elantech_data *etd = psmouse->private;
unsigned char *packet = psmouse->packet;
unsigned char packet_type = packet[3] & 0x03;
+ bool sanity_check;
+
+ /*
+ * Sanity check based on the constant bits of a packet.
+ * The constant bits change depending on the value of
+ * the hardware flag 'crc_enabled' but are the same for
+ * every packet, regardless of the type.
+ */
+ if (etd->crc_enabled)
+ sanity_check = ((packet[3] & 0x08) == 0x00);
+ else
+ sanity_check = ((packet[0] & 0x0c) == 0x04 &&
+ (packet[3] & 0x1c) == 0x10);
+
+ if (!sanity_check)
+ return PACKET_UNKNOWN;
switch (packet_type) {
case 0:
@@ -1313,6 +1343,12 @@ static int elantech_set_properties(struct elantech_data *etd)
etd->reports_pressure = true;
}
+ /*
+ * The signatures of v3 and v4 packets change depending on the
+ * value of this hardware flag.
+ */
+ etd->crc_enabled = ((etd->fw_version & 0x4000) == 0x4000);
+
return 0;
}
diff --git a/drivers/input/mouse/elantech.h b/drivers/input/mouse/elantech.h
index 46db3be..036a04a 100644
--- a/drivers/input/mouse/elantech.h
+++ b/drivers/input/mouse/elantech.h
@@ -129,6 +129,7 @@ struct elantech_data {
bool paritycheck;
bool jumpy_cursor;
bool reports_pressure;
+ bool crc_enabled;
unsigned char hw_version;
unsigned int fw_version;
unsigned int single_finger_reports;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 28/29] drivers/input/{keyboard,serio}: simplify use of devm_ioremap_resource
From: Julia Lawall @ 2013-08-14 11:03 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: kernel-janitors, linux-input, linux-kernel
From: Julia Lawall <Julia.Lawall@lip6.fr>
Remove unneeded error handling on the result of a call to
platform_get_resource when the value is passed to devm_ioremap_resource.
Move the call to platform_get_resource adjacent to the call to
devm_ioremap_resource to make the connection between them more clear.
A simplified version of the semantic patch that makes this change is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@@
expression pdev,res,n,e,e1;
expression ret != 0;
identifier l;
@@
- res = platform_get_resource(pdev, IORESOURCE_MEM, n);
... when != res
- if (res == NULL) { ... \(goto l;\|return ret;\) }
... when != res
+ res = platform_get_resource(pdev, IORESOURCE_MEM, n);
e = devm_ioremap_resource(e1, res);
// </smpl>
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/input/keyboard/imx_keypad.c | 7 +------
drivers/input/keyboard/nspire-keypad.c | 7 +------
drivers/input/keyboard/spear-keyboard.c | 7 +------
drivers/input/serio/arc_ps2.c | 7 +------
drivers/input/serio/olpc_apsp.c | 3 ---
5 files changed, 4 insertions(+), 27 deletions(-)
diff --git a/drivers/input/keyboard/imx_keypad.c b/drivers/input/keyboard/imx_keypad.c
index 03c8cc5..328cfc1 100644
--- a/drivers/input/keyboard/imx_keypad.c
+++ b/drivers/input/keyboard/imx_keypad.c
@@ -442,12 +442,6 @@ static int imx_keypad_probe(struct platform_device *pdev)
return -EINVAL;
}
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (res == NULL) {
- dev_err(&pdev->dev, "no I/O memory defined in platform data\n");
- return -EINVAL;
- }
-
input_dev = devm_input_allocate_device(&pdev->dev);
if (!input_dev) {
dev_err(&pdev->dev, "failed to allocate the input device\n");
@@ -468,6 +462,7 @@ static int imx_keypad_probe(struct platform_device *pdev)
setup_timer(&keypad->check_matrix_timer,
imx_keypad_check_for_events, (unsigned long) keypad);
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
keypad->mmio_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(keypad->mmio_base))
return PTR_ERR(keypad->mmio_base);
diff --git a/drivers/input/keyboard/nspire-keypad.c b/drivers/input/keyboard/nspire-keypad.c
index 20d872d..b3e3eda 100644
--- a/drivers/input/keyboard/nspire-keypad.c
+++ b/drivers/input/keyboard/nspire-keypad.c
@@ -171,12 +171,6 @@ static int nspire_keypad_probe(struct platform_device *pdev)
return -EINVAL;
}
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(&pdev->dev, "missing platform resources\n");
- return -EINVAL;
- }
-
keypad = devm_kzalloc(&pdev->dev, sizeof(struct nspire_keypad),
GFP_KERNEL);
if (!keypad) {
@@ -208,6 +202,7 @@ static int nspire_keypad_probe(struct platform_device *pdev)
return PTR_ERR(keypad->clk);
}
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
keypad->reg_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(keypad->reg_base))
return PTR_ERR(keypad->reg_base);
diff --git a/drivers/input/keyboard/spear-keyboard.c b/drivers/input/keyboard/spear-keyboard.c
index 7111124..85ff530 100644
--- a/drivers/input/keyboard/spear-keyboard.c
+++ b/drivers/input/keyboard/spear-keyboard.c
@@ -191,12 +191,6 @@ static int spear_kbd_probe(struct platform_device *pdev)
int irq;
int error;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(&pdev->dev, "no keyboard resource defined\n");
- return -EBUSY;
- }
-
irq = platform_get_irq(pdev, 0);
if (irq < 0) {
dev_err(&pdev->dev, "not able to get irq for the device\n");
@@ -228,6 +222,7 @@ static int spear_kbd_probe(struct platform_device *pdev)
kbd->suspended_rate = pdata->suspended_rate;
}
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
kbd->io_base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(kbd->io_base))
return PTR_ERR(kbd->io_base);
diff --git a/drivers/input/serio/arc_ps2.c b/drivers/input/serio/arc_ps2.c
index 3fb7727..8024a6d 100644
--- a/drivers/input/serio/arc_ps2.c
+++ b/drivers/input/serio/arc_ps2.c
@@ -189,12 +189,6 @@ static int arc_ps2_probe(struct platform_device *pdev)
int irq;
int error, id, i;
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(&pdev->dev, "no IO memory defined\n");
- return -EINVAL;
- }
-
irq = platform_get_irq_byname(pdev, "arc_ps2_irq");
if (irq < 0) {
dev_err(&pdev->dev, "no IRQ defined\n");
@@ -208,6 +202,7 @@ static int arc_ps2_probe(struct platform_device *pdev)
return -ENOMEM;
}
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
arc_ps2->addr = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(arc_ps2->addr))
return PTR_ERR(arc_ps2->addr);
diff --git a/drivers/input/serio/olpc_apsp.c b/drivers/input/serio/olpc_apsp.c
index 818aa46..51b1d40 100644
--- a/drivers/input/serio/olpc_apsp.c
+++ b/drivers/input/serio/olpc_apsp.c
@@ -183,9 +183,6 @@ static int olpc_apsp_probe(struct platform_device *pdev)
np = pdev->dev.of_node;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res)
- return -ENOENT;
-
priv->base = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(priv->base)) {
dev_err(&pdev->dev, "Failed to map WTM registers\n");
^ permalink raw reply related
* Re: [PATCH 28/29] drivers/input/{keyboard,serio}: simplify use of devm_ioremap_resource
From: Viresh Kumar @ 2013-08-14 11:21 UTC (permalink / raw)
To: Julia Lawall
Cc: Dmitry Torokhov, kernel-janitors, linux-input,
linux-kernel@vger.kernel.org, spear-devel
In-Reply-To: <1376478213-24091-1-git-send-email-Julia.Lawall@lip6.fr>
On Wed, Aug 14, 2013 at 4:33 PM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Remove unneeded error handling on the result of a call to
> platform_get_resource when the value is passed to devm_ioremap_resource.
>
> Move the call to platform_get_resource adjacent to the call to
> devm_ioremap_resource to make the connection between them more clear.
>
> A simplified version of the semantic patch that makes this change is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> expression pdev,res,n,e,e1;
> expression ret != 0;
> identifier l;
> @@
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, n);
> ... when != res
> - if (res == NULL) { ... \(goto l;\|return ret;\) }
> ... when != res
> + res = platform_get_resource(pdev, IORESOURCE_MEM, n);
> e = devm_ioremap_resource(e1, res);
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
> drivers/input/keyboard/spear-keyboard.c | 7 +------
For SPEAr:
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
^ permalink raw reply
* Re: kernel Oops: 0003 on usbhid_submit_report
From: Andreas Lillebø Holm @ 2013-08-14 13:38 UTC (permalink / raw)
To: Greg KH; +Cc: linux-usb, linux-input
In-Reply-To: <20130813184056.GH7740@kroah.com>
Hi,
On Tuesday, August 13, 2013 at 8:40 PM, Greg KH wrote:
> > When communicating with AT90USB1287, at random intervals (1/25 boots)
> > the linux hid_output_field Oopses and kills the communicating thread.
> > The AT90USB1287 microcontroller uses LUFA library for usb/hid
> > communication. It is trigged by a ioctl call from userspace and fails
> > in a kernel paging request. The system is after the oops in a state
> > where no hid commands is sent anymore and only a boot can fix the
> > system.
> >
> > Keywords: usbhid hid
> >
> > Kernel version: Linux version 3.8.13-03081301-generic (apw@gomeisa) (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) ) #201305311535 SMP Fri May 31 19:44:30 UTC 2013
> >
> > Oopses:
> > http://paste.debian.net/24305
> > http://paste.debian.net/24306
> > http://paste.debian.net/24307
> >
> > Code:
> > The error is triggered by:
> > ioctl(fd,HIDIOCSUSAGES, &ref_multi_u);
> > ioctl(fd,HIDIOCSREPORT, &rep_info_u);
> >
> > Notes:
> > It is very hard to reproduce so seems like race condition…
> >
> > Any tips to resolve/workaround this issue is appreciated and please
> > let me know if my information is incomplete (This is my first kernel
> > bug report)
>
>
>
>
> Any chance you can try a supported kernel, like 3.10.6 or 3.11-rc5 to
> see if that also causes problems? We can't do anything with
> distro-specific kernel releases like your 3.8.13 release from Ubuntu,
> sorry.
I've now tried with kernel 3.10.6 (http://kernel.ubuntu.com/~kernel-ppa/mainline/v3.10.6-saucy/) and I can trigger the same issue on this kernel.
Here is a paste from last Oops on this kernel:
http://paste.debian.net/24993/
I am also using usbmon to monitor the usb bus traffic, but cannot see anything that should cause the driver to Oops.
Is there any way to find out what can trigger this issue?
Thanks
Andreas L. Holm
--
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 1/3] HID: Use existing parser for pre-scanning the report descriptors
From: Benjamin Tissoires @ 2013-08-14 15:08 UTC (permalink / raw)
To: Alexander Holler
Cc: Benjamin Tissoires, Henrik Rydberg, Jiri Kosina, Stephane Chatty,
Mika Westerberg, linux-input, linux-kernel, Srinivas Pandruvada
In-Reply-To: <520A7CDF.3070808@ahsoftware.de>
On 13/08/13 20:37, Alexander Holler wrote:
> Am 13.08.2013 16:58, schrieb Benjamin Tissoires:
>> hid_scan_report() implements its own HID report descriptor parsing. It
>> was
>> fine until we added the SENSOR_HUB detection. It is going to be even
>> worse
>> with the detection of Win 8 certified touchscreen, as this detection
>> relies on a special feature and on the report_size and report_count
>> fields.
>
> Sorry, but I can't agree with your wording here.
>
> If you look at the if expression I've added to support HID sensor hubs,
> you will notice that the first expression was a test for the usage page
> of sensor hubs. That wasn't the first test by accident, instead I've
> choosen the order of those 4 tests very carefully to make the impact on
> the existing parsing of other HID devices very small. So it might not
> have be pleasing to your eyes, but it was for sure an appropriate solution.
>
> Anyway, I've tested your patch 1/3 on top of 3.11-rc5 here and the
> detection of sensor hubs doesn't work anymore with your patch applied.
> Up to now I only had a quick look at it, but it looks like the test in
> hid_scan_open_collection() isn't hit for my device.
Ok, I found the culprit (see below). Thanks for providing your report
descriptors (and sources), I managed to test the detection of your device.
>
> Another problem is that I don't have any commercial sensor hub and I'm
> therefor not a very relvant as tester (I've implemented the firmware for
> my HID (sensor hub) device too). Therefor I've added Srinivas Pandruvada
> to cc, because he's the only one I know who has HID sensor hubs.
>
> And, as said, I've implemented the other side here too, therefor I've
> added the descriptor I'm using below.
>
> Regards,
>
> Alexander Holler
>
>
> --my-descriptor--
> 0x05, 0x20, // HID_USAGE_PAGE_SENSOR
> 0xa1, 0x01, // COLLECTION (Application)
> 0x09, 0xa0, // HID_USAGE_SENSOR_CATEGORY_TIME
> 0xa1, 0x00, // COLLECTION (Physical)
> #ifndef USE_FULL_YEAR
> 0x75, 0x08, // REPORT_SIZE (8 bits)
> #endif
> 0x95, 0x01, // REPORT_COUNT (1)
> 0x85, HID_REPORT_ID_TIME, // REPORT_ID
>
> 0x0a, 0x21, 0x05, // USAGE (Year)
> #ifdef USE_FULL_YEAR
> 0x75, 0x10, // REPORT_SIZE (16 bits)
> #else
> 0x15, 0x00, // LOGICAL_MINIMUM (0) (Range is currently not
> specified in HUTRR39)
> 0x25, 0x63, // LOGICAL_MAXIMUM (99)
> #endif
> 0x65, 0x00, // UNIT (None)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0x0a, 0x22, 0x05, // USAGE (Month)
> #ifdef USE_FULL_YEAR
> 0x75, 0x08, // REPORT_SIZE (8 bits)
> #endif
> 0x15, 0x01, // LOGICAL_MINIMUM (1)
> 0x25, 0x0c, // LOGICAL_MAXIMUM (12)
> 0x65, 0x00, // UNIT (None)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0x0a, 0x23, 0x05, // USAGE (Day)
> 0x15, 0x01, // LOGICAL_MINIMUM (1)
> 0x25, 0x1f, // LOGICAL_MAXIMUM (31)
> 0x65, 0x00, // UNIT (None)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0x0a, 0x24, 0x05, // USAGE (Day of Week)
> 0x15, 0x00, // LOGICAL_MINIMUM (0)
> 0x25, 0x06, // LOGICAL_MAXIMUM (6)
> 0xa1, 0x02, // COLLECTION (Logical)
> 0x0a, 0xc0, 0x08, // Day of Week: Sunday
> 0x0a, 0xc1, 0x08, // Day of Week: Monday
> 0x0a, 0xc2, 0x08, // Day of Week: Tuesday
> 0x0a, 0xc3, 0x08, // Day of Week: Wednesday
> 0x0a, 0xc4, 0x08, // Day of Week: Thursday
> 0x0a, 0xc5, 0x08, // Day of Week: Friday
> 0x0a, 0xc6, 0x08, // Day of Week: Saturday
> 0x81, 0x02, // INPUT (Const,Arr,Abs)
> 0xc0, // END_COLLECTION
>
> 0x0a, 0x25, 0x05, // USAGE (Hour)
> 0x15, 0x00, // LOGICAL_MINIMUM (0)
> 0x25, 0x17, // LOGICAL_MAXIMUM (23)
> 0x65, 0x00, // UNIT (None)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0x0a, 0x26, 0x05, // USAGE (Minute)
> 0x15, 0x00, // LOGICAL_MINIMUM (0)
> 0x25, 0x3b, // LOGICAL_MAXIMUM (59)
> 0x65, 0x00, // UNIT (None)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0x0a, 0x27, 0x05, // USAGE (Second)
> 0x15, 0x00, // LOGICAL_MINIMUM (0)
> 0x25, 0x3b, // LOGICAL_MAXIMUM (59)
> 0x65, 0x00, // UNIT (None)
> //0x66, 0x10, 0x01, // UNIT (second)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0x0a, 0x28, 0x05, // USAGE (Millisecond)
> 0x75, 0x10, // REPORT_SIZE (16 bits)
> 0x65, 0x00, // UNIT (None)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0xc0, // END_COLLECTION
> 0xc0, // END_COLLECTION
> --my-descriptor--
>
>>
>> We can use the existing HID parser in hid-core for hid_scan_report()
>> by re-using the code from hid_open_report(). hid_parser_global,
>> hid_parser_local and hid_parser_reserved does not have any side effects.
>> We just need to reimplement the MAIN_ITEM callback to have a proper
>> parsing without side effects.
>>
>> Instead of directly overwriting the ->group field, this patch introduce
>> a ->flags field and then decide which group the device belongs to,
>> depending on the whole parsing (not just the local item). This will be
>> useful for Win 8 multitouch devices, which are multitouch devices and
>> Win 8 certified (so 2 flags to check).
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> ---
>> drivers/hid/hid-core.c | 131
>> +++++++++++++++++++++++++++++++++++--------------
>> include/linux/hid.h | 4 ++
>> 2 files changed, 97 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 3efe19f..d8cdb0a 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -677,10 +677,49 @@ static u8 *fetch_item(__u8 *start, __u8 *end,
>> struct hid_item *item)
>> return NULL;
>> }
>>
>> -static void hid_scan_usage(struct hid_device *hid, u32 usage)
>> +static void hid_scan_input_usage(struct hid_parser *parser, u32 usage)
>> {
>> if (usage == HID_DG_CONTACTID)
>> - hid->group = HID_GROUP_MULTITOUCH;
>> + parser->flags |= HID_FLAG_MULTITOUCH;
>> +}
>> +
>> +static void hid_scan_open_collection(struct hid_parser *parser,
>> unsigned type)
>> +{
>> + if (parser->global.usage_page == HID_UP_SENSOR &&
HID_UP_SENSOR is actually 0x00200000, and parser->global.usage_page
contains the raw value 0x0020... Moving HID_UP_SENSOR 16 bits to the
right makes the test working, and your sensor device correctly tagged.
Cheers,
Benjamin
>> + type == HID_COLLECTION_PHYSICAL)
>> + parser->flags |= HID_FLAG_SENSOR_HUB;
>> +}
>> +
>> +static int hid_scan_main(struct hid_parser *parser, struct hid_item
>> *item)
>> +{
>> + __u32 data;
>> + int i;
>> +
>> + data = item_udata(item);
>> +
>> + switch (item->tag) {
>> + case HID_MAIN_ITEM_TAG_BEGIN_COLLECTION:
>> + hid_scan_open_collection(parser, data & 0xff);
>> + break;
>> + case HID_MAIN_ITEM_TAG_END_COLLECTION:
>> + break;
>> + case HID_MAIN_ITEM_TAG_INPUT:
>> + for (i = 0; i < parser->local.usage_index; i++)
>> + hid_scan_input_usage(parser, parser->local.usage[i]);
>> + break;
>> + case HID_MAIN_ITEM_TAG_OUTPUT:
>> + break;
>> + case HID_MAIN_ITEM_TAG_FEATURE:
>> + break;
>> + default:
>> + hid_err(parser->device, "unknown main item tag 0x%x\n",
>> + item->tag);
>> + }
>> +
>> + /* Reset the local parser environment */
>> + memset(&parser->local, 0, sizeof(parser->local));
>> +
>> + return 0;
>> }
>>
>> /*
>> @@ -690,49 +729,65 @@ static void hid_scan_usage(struct hid_device
>> *hid, u32 usage)
>> */
>> static int hid_scan_report(struct hid_device *hid)
>> {
>> - unsigned int page = 0, delim = 0;
>> + struct hid_parser *parser;
>> + struct hid_item item;
>> __u8 *start = hid->dev_rdesc;
>> __u8 *end = start + hid->dev_rsize;
>> - unsigned int u, u_min = 0, u_max = 0;
>> - struct hid_item item;
>> + int ret;
>> + static int (*dispatch_type[])(struct hid_parser *parser,
>> + struct hid_item *item) = {
>> + hid_scan_main,
>> + hid_parser_global,
>> + hid_parser_local,
>> + hid_parser_reserved
>> + };
>>
>> - hid->group = HID_GROUP_GENERIC;
>> + parser = vzalloc(sizeof(struct hid_parser));
>> + if (!parser)
>> + return -ENOMEM;
>> +
>> + parser->device = hid;
>> +
>> + ret = -EINVAL;
>> while ((start = fetch_item(start, end, &item)) != NULL) {
>> - if (item.format != HID_ITEM_FORMAT_SHORT)
>> - return -EINVAL;
>> - if (item.type == HID_ITEM_TYPE_GLOBAL) {
>> - if (item.tag == HID_GLOBAL_ITEM_TAG_USAGE_PAGE)
>> - page = item_udata(&item) << 16;
>> - } else if (item.type == HID_ITEM_TYPE_LOCAL) {
>> - if (delim > 1)
>> - break;
>> - u = item_udata(&item);
>> - if (item.size <= 2)
>> - u += page;
>> - switch (item.tag) {
>> - case HID_LOCAL_ITEM_TAG_DELIMITER:
>> - delim += !!u;
>> - break;
>> - case HID_LOCAL_ITEM_TAG_USAGE:
>> - hid_scan_usage(hid, u);
>> - break;
>> - case HID_LOCAL_ITEM_TAG_USAGE_MINIMUM:
>> - u_min = u;
>> - break;
>> - case HID_LOCAL_ITEM_TAG_USAGE_MAXIMUM:
>> - u_max = u;
>> - for (u = u_min; u <= u_max; u++)
>> - hid_scan_usage(hid, u);
>> - break;
>> +
>> + if (item.format != HID_ITEM_FORMAT_SHORT) {
>> + hid_err(hid, "unexpected long global item\n");
>> + goto out;
>> + }
>> +
>> + if (dispatch_type[item.type](parser, &item)) {
>> + hid_err(hid, "item %u %u %u %u parsing failed\n",
>> + item.format, (unsigned)item.size,
>> + (unsigned)item.type, (unsigned)item.tag);
>> + goto out;
>> + }
>> +
>> + if (start == end) {
>> + if (parser->local.delimiter_depth) {
>> + hid_err(hid, "unbalanced delimiter at end of report
>> description\n");
>> + goto out;
>> }
>> - } else if (page == HID_UP_SENSOR &&
>> - item.type == HID_ITEM_TYPE_MAIN &&
>> - item.tag == HID_MAIN_ITEM_TAG_BEGIN_COLLECTION &&
>> - (item_udata(&item) & 0xff) == HID_COLLECTION_PHYSICAL)
>> - hid->group = HID_GROUP_SENSOR_HUB;
>> + ret = 0;
>> + goto out;
>> + }
>> }
>>
>> - return 0;
>> + hid_err(hid, "item fetching failed at offset %d\n", (int)(end -
>> start));
>> +out:
>> + switch (parser->flags) {
>> + case HID_FLAG_MULTITOUCH:
>> + hid->group = HID_GROUP_MULTITOUCH;
>> + break;
>> + case HID_FLAG_SENSOR_HUB:
>> + hid->group = HID_GROUP_SENSOR_HUB;
>> + break;
>> + default:
>> + hid->group = HID_GROUP_GENERIC;
>> + }
>> +
>> + vfree(parser);
>> + return ret;
>> }
>>
>> /**
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index 5a4e789..7d823db 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -533,6 +533,9 @@ static inline void hid_set_drvdata(struct
>> hid_device *hdev, void *data)
>> #define HID_GLOBAL_STACK_SIZE 4
>> #define HID_COLLECTION_STACK_SIZE 4
>>
>> +#define HID_FLAG_MULTITOUCH 0x0001
>> +#define HID_FLAG_SENSOR_HUB 0x0002
>> +
>> struct hid_parser {
>> struct hid_global global;
>> struct hid_global global_stack[HID_GLOBAL_STACK_SIZE];
>> @@ -541,6 +544,7 @@ struct hid_parser {
>> unsigned collection_stack[HID_COLLECTION_STACK_SIZE];
>> unsigned collection_stack_ptr;
>> struct hid_device *device;
>> + unsigned flags;
>> };
>>
>> struct hid_class_descriptor {
>>
>
^ permalink raw reply
* Re: [PATCH 1/3] HID: Use existing parser for pre-scanning the report descriptors
From: Benjamin Tissoires @ 2013-08-14 15:38 UTC (permalink / raw)
To: rydberg
Cc: Benjamin Tissoires, Jiri Kosina, Stephane Chatty, Mika Westerberg,
Alexander Holler, linux-input, linux-kernel
In-Reply-To: <20130813191731.GA8391@polaris.bitmath.org>
Hi Henrik,
On 13/08/13 21:17, rydberg@euromail.se wrote:
> Hi Benjamin,
>
> thanks for the patches, things are looking a lot better this way.
thanks for the review :)
>
>> hid_scan_report() implements its own HID report descriptor parsing. It was
>> fine until we added the SENSOR_HUB detection. It is going to be even worse
>> with the detection of Win 8 certified touchscreen, as this detection
>> relies on a special feature and on the report_size and report_count fields.
>
> It was fine with sensors added as well. You seem to have found a
> reasonable way to add support for all the tags, but there is a
> rationale for the current scanner that may not have been addressed in
> this patch: it is robust against parse errors. This is particularly
> important for devices which later tweak the report, often in order to
> parse properly.
Hmm, I disagree: if a driver needs to tweak a report, then it will
register itself in hid_have_special_driver. The pre-scanning pass is
done only for devices which are not in hid_have_special_driver. So I
consider it is safe to assume that the report descriptor does not
contain errors. If it does, then it will just leave the current
hid-group state, and will just pop an debug information.
>
> Please find some further comments inline.
>
>> We can use the existing HID parser in hid-core for hid_scan_report()
>> by re-using the code from hid_open_report(). hid_parser_global,
>> hid_parser_local and hid_parser_reserved does not have any side effects.
>> We just need to reimplement the MAIN_ITEM callback to have a proper
>> parsing without side effects.
>>
>> Instead of directly overwriting the ->group field, this patch introduce
>> a ->flags field and then decide which group the device belongs to,
>> depending on the whole parsing (not just the local item). This will be
>> useful for Win 8 multitouch devices, which are multitouch devices and
>> Win 8 certified (so 2 flags to check).
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> ---
>> drivers/hid/hid-core.c | 131 +++++++++++++++++++++++++++++++++++--------------
>> include/linux/hid.h | 4 ++
>> 2 files changed, 97 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 3efe19f..d8cdb0a 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -677,10 +677,49 @@ static u8 *fetch_item(__u8 *start, __u8 *end, struct hid_item *item)
>> return NULL;
>> }
>>
>> -static void hid_scan_usage(struct hid_device *hid, u32 usage)
>> +static void hid_scan_input_usage(struct hid_parser *parser, u32 usage)
>> {
>> if (usage == HID_DG_CONTACTID)
>> - hid->group = HID_GROUP_MULTITOUCH;
>> + parser->flags |= HID_FLAG_MULTITOUCH;
>
> Did you consider reusing the group flags, e.g., parser->groups |= (1
> << HID_GROUP_MULTITOUCH)? This change could be made regardless of the
> parser logic.
If nobody is against changing the values of the different groups across
kernel version (which should be harmless), then I fully agree, we can
use the group item as a bit field (but we would be able to only have 16
different device groups).
>
>> +}
>> +
>> +static void hid_scan_open_collection(struct hid_parser *parser, unsigned type)
>
> We are not really opening anything here, so perhaps
> hid_scan_collection would suffice.
Ok, will change in v2
>
>> +{
>> + if (parser->global.usage_page == HID_UP_SENSOR &&
>> + type == HID_COLLECTION_PHYSICAL)
>> + parser->flags |= HID_FLAG_SENSOR_HUB;
>> +}
>> +
>> +static int hid_scan_main(struct hid_parser *parser, struct hid_item *item)
>> +{
>> + __u32 data;
>> + int i;
>> +
>> + data = item_udata(item);
>> +
>> + switch (item->tag) {
>> + case HID_MAIN_ITEM_TAG_BEGIN_COLLECTION:
>> + hid_scan_open_collection(parser, data & 0xff);
>> + break;
>> + case HID_MAIN_ITEM_TAG_END_COLLECTION:
>> + break;
>> + case HID_MAIN_ITEM_TAG_INPUT:
>> + for (i = 0; i < parser->local.usage_index; i++)
>> + hid_scan_input_usage(parser, parser->local.usage[i]);
>> + break;
>> + case HID_MAIN_ITEM_TAG_OUTPUT:
>> + break;
>> + case HID_MAIN_ITEM_TAG_FEATURE:
>> + break;
>> + default:
>> + hid_err(parser->device, "unknown main item tag 0x%x\n",
>> + item->tag);
>> + }
>> +
>> + /* Reset the local parser environment */
>> + memset(&parser->local, 0, sizeof(parser->local));
>> +
>> + return 0;
>> }
>>
>> /*
>> @@ -690,49 +729,65 @@ static void hid_scan_usage(struct hid_device *hid, u32 usage)
>> */
>> static int hid_scan_report(struct hid_device *hid)
>> {
>> - unsigned int page = 0, delim = 0;
>> + struct hid_parser *parser;
>> + struct hid_item item;
>> __u8 *start = hid->dev_rdesc;
>> __u8 *end = start + hid->dev_rsize;
>> - unsigned int u, u_min = 0, u_max = 0;
>> - struct hid_item item;
>> + int ret;
>> + static int (*dispatch_type[])(struct hid_parser *parser,
>> + struct hid_item *item) = {
>> + hid_scan_main,
>> + hid_parser_global,
>> + hid_parser_local,
>> + hid_parser_reserved
>> + };
>>
>> - hid->group = HID_GROUP_GENERIC;
>> + parser = vzalloc(sizeof(struct hid_parser));
>
> Argh, I realize it is inevitable for this patch, but it still makes my
> eyes bleed. The parser takes quite a bit of memory...
Yep, my first attempt was to remove it, then I re-added it with a small
tear...
>
>> + if (!parser)
>> + return -ENOMEM;
>> +
>> + parser->device = hid;
>> +
>> + ret = -EINVAL;
>> while ((start = fetch_item(start, end, &item)) != NULL) {
>> - if (item.format != HID_ITEM_FORMAT_SHORT)
>> - return -EINVAL;
>> - if (item.type == HID_ITEM_TYPE_GLOBAL) {
>> - if (item.tag == HID_GLOBAL_ITEM_TAG_USAGE_PAGE)
>> - page = item_udata(&item) << 16;
>> - } else if (item.type == HID_ITEM_TYPE_LOCAL) {
>> - if (delim > 1)
>> - break;
>> - u = item_udata(&item);
>> - if (item.size <= 2)
>> - u += page;
>> - switch (item.tag) {
>> - case HID_LOCAL_ITEM_TAG_DELIMITER:
>> - delim += !!u;
>> - break;
>> - case HID_LOCAL_ITEM_TAG_USAGE:
>> - hid_scan_usage(hid, u);
>> - break;
>> - case HID_LOCAL_ITEM_TAG_USAGE_MINIMUM:
>> - u_min = u;
>> - break;
>> - case HID_LOCAL_ITEM_TAG_USAGE_MAXIMUM:
>> - u_max = u;
>> - for (u = u_min; u <= u_max; u++)
>> - hid_scan_usage(hid, u);
>> - break;
>> +
>> + if (item.format != HID_ITEM_FORMAT_SHORT) {
>> + hid_err(hid, "unexpected long global item\n");
>
> I do not think we should be verbose on errors during scan, for the
> reason stated at the top. Since this goes also for the global parser
> functions, we might have a problem.
yes, I kept it for documenting purposes, but this will double the amount
of logs in the kernel debug output. I'll just convert them into comments
in v2.
>
>> + goto out;
>> + }
>> +
>> + if (dispatch_type[item.type](parser, &item)) {
>> + hid_err(hid, "item %u %u %u %u parsing failed\n",
>> + item.format, (unsigned)item.size,
>> + (unsigned)item.type, (unsigned)item.tag);
>
> Ditto.
Ditto
>
>> + goto out;
>> + }
>> +
>> + if (start == end) {
>> + if (parser->local.delimiter_depth) {
>> + hid_err(hid, "unbalanced delimiter at end of report description\n");
>
> Robustness, see top.
>
>> + goto out;
>> }
>> - } else if (page == HID_UP_SENSOR &&
>> - item.type == HID_ITEM_TYPE_MAIN &&
>> - item.tag == HID_MAIN_ITEM_TAG_BEGIN_COLLECTION &&
>> - (item_udata(&item) & 0xff) == HID_COLLECTION_PHYSICAL)
>> - hid->group = HID_GROUP_SENSOR_HUB;
>
> At the end of the day, It may be best to simply extend this branch as
> the main item type and add whatever you need to detect win8 from
> there.
>
I don't think this would be that simple:
- the usage is given by the HID_ITEM_TYPE_LOCAL branch
- I also need to check the fact that the usage is used as a feature:
this would require setting temporary flags and involve a logic which
would not be very easy to understand.
Furthermore, this makes me think that if a device describes in the
reports a ContactID as a feature, then the current parsing will say that
this is a multitouch device, whereas it should not. (ok, this has no
reasons to appear because it would be dumb, but still...)
>> + ret = 0;
>> + goto out;
>> + }
>> }
>>
>> - return 0;
>> + hid_err(hid, "item fetching failed at offset %d\n", (int)(end - start));
>> +out:
>> + switch (parser->flags) {
>> + case HID_FLAG_MULTITOUCH:
>> + hid->group = HID_GROUP_MULTITOUCH;
>> + break;
>> + case HID_FLAG_SENSOR_HUB:
>> + hid->group = HID_GROUP_SENSOR_HUB;
>> + break;
>> + default:
>> + hid->group = HID_GROUP_GENERIC;
>> + }
>
> Looks odd to switch on flags, but it works pretty well with the rest
> of the patches in the series.
Yep, it's odd. If nobody is against changing the current groups
definitions, we can just keep the group as a bitfield, and it will work
(we just need to sanitize the group at the end).
Cheers,
Benjamin
>
>> +
>> + vfree(parser);
>> + return ret;
>> }
>>
>> /**
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index 5a4e789..7d823db 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -533,6 +533,9 @@ static inline void hid_set_drvdata(struct hid_device *hdev, void *data)
>> #define HID_GLOBAL_STACK_SIZE 4
>> #define HID_COLLECTION_STACK_SIZE 4
>>
>> +#define HID_FLAG_MULTITOUCH 0x0001
>> +#define HID_FLAG_SENSOR_HUB 0x0002
>> +
>> struct hid_parser {
>> struct hid_global global;
>> struct hid_global global_stack[HID_GLOBAL_STACK_SIZE];
>> @@ -541,6 +544,7 @@ struct hid_parser {
>> unsigned collection_stack[HID_COLLECTION_STACK_SIZE];
>> unsigned collection_stack_ptr;
>> struct hid_device *device;
>> + unsigned flags;
>> };
>>
>> struct hid_class_descriptor {
>> --
>> 1.8.3.1
>>
>
> Thanks,
> Henrik
>
^ permalink raw reply
* Re: [PATCH 1/3] HID: Use existing parser for pre-scanning the report descriptors
From: Srinivas Pandruvada @ 2013-08-14 16:07 UTC (permalink / raw)
To: Alexander Holler
Cc: Benjamin Tissoires, Benjamin Tissoires, Henrik Rydberg,
Jiri Kosina, Stephane Chatty, Mika Westerberg, linux-input,
linux-kernel
In-Reply-To: <520A7CDF.3070808@ahsoftware.de>
On 08/13/2013 11:37 AM, Alexander Holler wrote:
> Am 13.08.2013 16:58, schrieb Benjamin Tissoires:
>> hid_scan_report() implements its own HID report descriptor parsing.
>> It was
>> fine until we added the SENSOR_HUB detection. It is going to be even
>> worse
>> with the detection of Win 8 certified touchscreen, as this detection
>> relies on a special feature and on the report_size and report_count
>> fields.
>
> Sorry, but I can't agree with your wording here.
>
> If you look at the if expression I've added to support HID sensor
> hubs, you will notice that the first expression was a test for the
> usage page of sensor hubs. That wasn't the first test by accident,
> instead I've choosen the order of those 4 tests very carefully to make
> the impact on the existing parsing of other HID devices very small. So
> it might not have be pleasing to your eyes, but it was for sure an
> appropriate solution.
>
> Anyway, I've tested your patch 1/3 on top of 3.11-rc5 here and the
> detection of sensor hubs doesn't work anymore with your patch applied.
> Up to now I only had a quick look at it, but it looks like the test in
> hid_scan_open_collection() isn't hit for my device.
>
> Another problem is that I don't have any commercial sensor hub and I'm
> therefor not a very relvant as tester (I've implemented the firmware
> for my HID (sensor hub) device too). Therefor I've added Srinivas
> Pandruvada to cc, because he's the only one I know who has HID sensor
> hubs.
>
Tested on 3.11-rc5. Sensor hubs are no longer detected. If you have some
patch to fix this, I can give a try.
> And, as said, I've implemented the other side here too, therefor I've
> added the descriptor I'm using below.
>
> Regards,
>
> Alexander Holler
>
>
> --my-descriptor--
> 0x05, 0x20, // HID_USAGE_PAGE_SENSOR
> 0xa1, 0x01, // COLLECTION (Application)
> 0x09, 0xa0, // HID_USAGE_SENSOR_CATEGORY_TIME
> 0xa1, 0x00, // COLLECTION (Physical)
> #ifndef USE_FULL_YEAR
> 0x75, 0x08, // REPORT_SIZE (8 bits)
> #endif
> 0x95, 0x01, // REPORT_COUNT (1)
> 0x85, HID_REPORT_ID_TIME, // REPORT_ID
>
> 0x0a, 0x21, 0x05, // USAGE (Year)
> #ifdef USE_FULL_YEAR
> 0x75, 0x10, // REPORT_SIZE (16 bits)
> #else
> 0x15, 0x00, // LOGICAL_MINIMUM (0) (Range is currently not
> specified in HUTRR39)
> 0x25, 0x63, // LOGICAL_MAXIMUM (99)
> #endif
> 0x65, 0x00, // UNIT (None)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0x0a, 0x22, 0x05, // USAGE (Month)
> #ifdef USE_FULL_YEAR
> 0x75, 0x08, // REPORT_SIZE (8 bits)
> #endif
> 0x15, 0x01, // LOGICAL_MINIMUM (1)
> 0x25, 0x0c, // LOGICAL_MAXIMUM (12)
> 0x65, 0x00, // UNIT (None)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0x0a, 0x23, 0x05, // USAGE (Day)
> 0x15, 0x01, // LOGICAL_MINIMUM (1)
> 0x25, 0x1f, // LOGICAL_MAXIMUM (31)
> 0x65, 0x00, // UNIT (None)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0x0a, 0x24, 0x05, // USAGE (Day of Week)
> 0x15, 0x00, // LOGICAL_MINIMUM (0)
> 0x25, 0x06, // LOGICAL_MAXIMUM (6)
> 0xa1, 0x02, // COLLECTION (Logical)
> 0x0a, 0xc0, 0x08, // Day of Week: Sunday
> 0x0a, 0xc1, 0x08, // Day of Week: Monday
> 0x0a, 0xc2, 0x08, // Day of Week: Tuesday
> 0x0a, 0xc3, 0x08, // Day of Week: Wednesday
> 0x0a, 0xc4, 0x08, // Day of Week: Thursday
> 0x0a, 0xc5, 0x08, // Day of Week: Friday
> 0x0a, 0xc6, 0x08, // Day of Week: Saturday
> 0x81, 0x02, // INPUT (Const,Arr,Abs)
> 0xc0, // END_COLLECTION
>
> 0x0a, 0x25, 0x05, // USAGE (Hour)
> 0x15, 0x00, // LOGICAL_MINIMUM (0)
> 0x25, 0x17, // LOGICAL_MAXIMUM (23)
> 0x65, 0x00, // UNIT (None)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0x0a, 0x26, 0x05, // USAGE (Minute)
> 0x15, 0x00, // LOGICAL_MINIMUM (0)
> 0x25, 0x3b, // LOGICAL_MAXIMUM (59)
> 0x65, 0x00, // UNIT (None)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0x0a, 0x27, 0x05, // USAGE (Second)
> 0x15, 0x00, // LOGICAL_MINIMUM (0)
> 0x25, 0x3b, // LOGICAL_MAXIMUM (59)
> 0x65, 0x00, // UNIT (None)
> //0x66, 0x10, 0x01, // UNIT (second)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0x0a, 0x28, 0x05, // USAGE (Millisecond)
> 0x75, 0x10, // REPORT_SIZE (16 bits)
> 0x65, 0x00, // UNIT (None)
> 0x81, 0x02, // INPUT (Data,Var,Abs)
>
> 0xc0, // END_COLLECTION
> 0xc0, // END_COLLECTION
> --my-descriptor--
>
>>
>> We can use the existing HID parser in hid-core for hid_scan_report()
>> by re-using the code from hid_open_report(). hid_parser_global,
>> hid_parser_local and hid_parser_reserved does not have any side effects.
>> We just need to reimplement the MAIN_ITEM callback to have a proper
>> parsing without side effects.
>>
>> Instead of directly overwriting the ->group field, this patch introduce
>> a ->flags field and then decide which group the device belongs to,
>> depending on the whole parsing (not just the local item). This will be
>> useful for Win 8 multitouch devices, which are multitouch devices and
>> Win 8 certified (so 2 flags to check).
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>> ---
>> drivers/hid/hid-core.c | 131
>> +++++++++++++++++++++++++++++++++++--------------
>> include/linux/hid.h | 4 ++
>> 2 files changed, 97 insertions(+), 38 deletions(-)
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 3efe19f..d8cdb0a 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -677,10 +677,49 @@ static u8 *fetch_item(__u8 *start, __u8 *end,
>> struct hid_item *item)
>> return NULL;
>> }
>>
>> -static void hid_scan_usage(struct hid_device *hid, u32 usage)
>> +static void hid_scan_input_usage(struct hid_parser *parser, u32 usage)
>> {
>> if (usage == HID_DG_CONTACTID)
>> - hid->group = HID_GROUP_MULTITOUCH;
>> + parser->flags |= HID_FLAG_MULTITOUCH;
>> +}
>> +
>> +static void hid_scan_open_collection(struct hid_parser *parser,
>> unsigned type)
>> +{
>> + if (parser->global.usage_page == HID_UP_SENSOR &&
>> + type == HID_COLLECTION_PHYSICAL)
>> + parser->flags |= HID_FLAG_SENSOR_HUB;
>> +}
>> +
>> +static int hid_scan_main(struct hid_parser *parser, struct hid_item
>> *item)
>> +{
>> + __u32 data;
>> + int i;
>> +
>> + data = item_udata(item);
>> +
>> + switch (item->tag) {
>> + case HID_MAIN_ITEM_TAG_BEGIN_COLLECTION:
>> + hid_scan_open_collection(parser, data & 0xff);
>> + break;
>> + case HID_MAIN_ITEM_TAG_END_COLLECTION:
>> + break;
>> + case HID_MAIN_ITEM_TAG_INPUT:
>> + for (i = 0; i < parser->local.usage_index; i++)
>> + hid_scan_input_usage(parser, parser->local.usage[i]);
>> + break;
>> + case HID_MAIN_ITEM_TAG_OUTPUT:
>> + break;
>> + case HID_MAIN_ITEM_TAG_FEATURE:
>> + break;
>> + default:
>> + hid_err(parser->device, "unknown main item tag 0x%x\n",
>> + item->tag);
>> + }
>> +
>> + /* Reset the local parser environment */
>> + memset(&parser->local, 0, sizeof(parser->local));
>> +
>> + return 0;
>> }
>>
>> /*
>> @@ -690,49 +729,65 @@ static void hid_scan_usage(struct hid_device
>> *hid, u32 usage)
>> */
>> static int hid_scan_report(struct hid_device *hid)
>> {
>> - unsigned int page = 0, delim = 0;
>> + struct hid_parser *parser;
>> + struct hid_item item;
>> __u8 *start = hid->dev_rdesc;
>> __u8 *end = start + hid->dev_rsize;
>> - unsigned int u, u_min = 0, u_max = 0;
>> - struct hid_item item;
>> + int ret;
>> + static int (*dispatch_type[])(struct hid_parser *parser,
>> + struct hid_item *item) = {
>> + hid_scan_main,
>> + hid_parser_global,
>> + hid_parser_local,
>> + hid_parser_reserved
>> + };
>>
>> - hid->group = HID_GROUP_GENERIC;
>> + parser = vzalloc(sizeof(struct hid_parser));
>> + if (!parser)
>> + return -ENOMEM;
>> +
>> + parser->device = hid;
>> +
>> + ret = -EINVAL;
>> while ((start = fetch_item(start, end, &item)) != NULL) {
>> - if (item.format != HID_ITEM_FORMAT_SHORT)
>> - return -EINVAL;
>> - if (item.type == HID_ITEM_TYPE_GLOBAL) {
>> - if (item.tag == HID_GLOBAL_ITEM_TAG_USAGE_PAGE)
>> - page = item_udata(&item) << 16;
>> - } else if (item.type == HID_ITEM_TYPE_LOCAL) {
>> - if (delim > 1)
>> - break;
>> - u = item_udata(&item);
>> - if (item.size <= 2)
>> - u += page;
>> - switch (item.tag) {
>> - case HID_LOCAL_ITEM_TAG_DELIMITER:
>> - delim += !!u;
>> - break;
>> - case HID_LOCAL_ITEM_TAG_USAGE:
>> - hid_scan_usage(hid, u);
>> - break;
>> - case HID_LOCAL_ITEM_TAG_USAGE_MINIMUM:
>> - u_min = u;
>> - break;
>> - case HID_LOCAL_ITEM_TAG_USAGE_MAXIMUM:
>> - u_max = u;
>> - for (u = u_min; u <= u_max; u++)
>> - hid_scan_usage(hid, u);
>> - break;
>> +
>> + if (item.format != HID_ITEM_FORMAT_SHORT) {
>> + hid_err(hid, "unexpected long global item\n");
>> + goto out;
>> + }
>> +
>> + if (dispatch_type[item.type](parser, &item)) {
>> + hid_err(hid, "item %u %u %u %u parsing failed\n",
>> + item.format, (unsigned)item.size,
>> + (unsigned)item.type, (unsigned)item.tag);
>> + goto out;
>> + }
>> +
>> + if (start == end) {
>> + if (parser->local.delimiter_depth) {
>> + hid_err(hid, "unbalanced delimiter at end of report
>> description\n");
>> + goto out;
>> }
>> - } else if (page == HID_UP_SENSOR &&
>> - item.type == HID_ITEM_TYPE_MAIN &&
>> - item.tag == HID_MAIN_ITEM_TAG_BEGIN_COLLECTION &&
>> - (item_udata(&item) & 0xff) == HID_COLLECTION_PHYSICAL)
>> - hid->group = HID_GROUP_SENSOR_HUB;
>> + ret = 0;
>> + goto out;
>> + }
>> }
>>
>> - return 0;
>> + hid_err(hid, "item fetching failed at offset %d\n", (int)(end -
>> start));
>> +out:
>> + switch (parser->flags) {
>> + case HID_FLAG_MULTITOUCH:
>> + hid->group = HID_GROUP_MULTITOUCH;
>> + break;
>> + case HID_FLAG_SENSOR_HUB:
>> + hid->group = HID_GROUP_SENSOR_HUB;
>> + break;
>> + default:
>> + hid->group = HID_GROUP_GENERIC;
>> + }
>> +
>> + vfree(parser);
>> + return ret;
>> }
>>
>> /**
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index 5a4e789..7d823db 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -533,6 +533,9 @@ static inline void hid_set_drvdata(struct
>> hid_device *hdev, void *data)
>> #define HID_GLOBAL_STACK_SIZE 4
>> #define HID_COLLECTION_STACK_SIZE 4
>>
>> +#define HID_FLAG_MULTITOUCH 0x0001
>> +#define HID_FLAG_SENSOR_HUB 0x0002
>> +
>> struct hid_parser {
>> struct hid_global global;
>> struct hid_global global_stack[HID_GLOBAL_STACK_SIZE];
>> @@ -541,6 +544,7 @@ struct hid_parser {
>> unsigned collection_stack[HID_COLLECTION_STACK_SIZE];
>> unsigned collection_stack_ptr;
>> struct hid_device *device;
>> + unsigned flags;
>> };
>>
>> struct hid_class_descriptor {
>>
>
>
Thanks,
Srinivas
^ permalink raw reply
* hid-debug and CONFIG_DEBUG_FS
From: Srinivas Pandruvada @ 2013-08-14 16:16 UTC (permalink / raw)
To: linux-input, Jiri Kosina
Hi Jiri,
I am implementing some hid driver which require relatively higher
throughput. HID debug relies on CONFIG_DEBUG_FS, which is by default
turned on by many distros. These debugs causes issues with the
throughput, because each report and event is logged.
What do you think about using something other than CONFIG_DEBUG_FS for
enabling HID debug (something like CONFIG_HID_DEBUG_FS..)?
Thanks,
Srinivas
^ permalink raw reply
* Re: [PATCH 0/4] iio: input: ti_am335x_adc: Add continuous sampling support round 5
From: Sebastian Andrzej Siewior @ 2013-08-14 18:05 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Zubair Lutfullah, jic23, dmitry.torokhov, linux-iio, linux-input,
linux-kernel, gregkh, Russ.Dill
In-Reply-To: <520BD308.7030709@kernel.org>
On 08/14/2013 08:57 PM, Jonathan Cameron wrote:
> I'll actually look at these later, but please do put a version number
me, too.
> Jonathan
Sebastian
^ permalink raw reply
* Re: [PATCH 0/4] iio: input: ti_am335x_adc: Add continuous sampling support round 5
From: Jonathan Cameron @ 2013-08-14 18:57 UTC (permalink / raw)
To: Zubair Lutfullah
Cc: jic23, dmitry.torokhov, linux-iio, linux-input, linux-kernel,
bigeasy, gregkh, Russ.Dill
In-Reply-To: <1376424303-22740-1-git-send-email-zubair.lutfullah@gmail.com>
I'll actually look at these later, but please do put a version number
in your cover letter patch subject.
[PATCH V5 0/4] is pretty much the convention to reduce the chance of anyone
looking at the wrong version.
Thanks,
Jonathan
On 08/13/13 21:04, Zubair Lutfullah wrote:
> Round 5 updates. Fixed the define order in the header as guided by Lee.
>
> Round 4 updates below.
>
> Note: These apply to the fixes-togreg branch in IIO because of
> a fix on the adc side in there.
>
> The first few are for input which tweak the TSC driver to
> allow ADC in continuous mode with irqs to work nicely together.
>
> The last one is for iio which adds continuous mode to the adc side.
>
> Received feedback on previous series.
> 1. What if IRQs occur together?
> This is handled now. Even if both assert.
> They both work.
>
> 2. IIO error handling wrong.
> Fixed now.
>
> 3. Headers wierd in IIO.
> Fixed.
>
> Apart from that, found a few bugs in continuous mode
> here and there. And squashed them into the same patch.
>
> Thanks
> Zubair Lutfullah
>
> Zubair Lutfullah (4):
> input: ti_am335x_tsc: correct step mask update after IRQ
> input: ti_am335x_tsc: Increase sequencer delay time
> input: ti_am335x_tsc: Enable shared IRQ for TSC, add overrun and
> underflow checks
> iio: ti_am335x_adc: Add continuous sampling and trigger support
>
> drivers/iio/adc/ti_am335x_adc.c | 353 ++++++++++++++++++++++++-----
> drivers/input/touchscreen/ti_am335x_tsc.c | 45 +++-
> include/linux/mfd/ti_am335x_tscadc.h | 14 ++
> 3 files changed, 342 insertions(+), 70 deletions(-)
>
^ permalink raw reply
* Re: [PATCH 1/3] HID: Use existing parser for pre-scanning the report descriptors
From: Alexander Holler @ 2013-08-14 20:03 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: rydberg, Benjamin Tissoires, Jiri Kosina, Stephane Chatty,
Mika Westerberg, linux-input, linux-kernel
In-Reply-To: <520BA464.9060200@redhat.com>
Am 14.08.2013 17:38, schrieb Benjamin Tissoires:
>>> {
>>> if (usage == HID_DG_CONTACTID)
>>> - hid->group = HID_GROUP_MULTITOUCH;
>>> + parser->flags |= HID_FLAG_MULTITOUCH;
>>
>> Did you consider reusing the group flags, e.g., parser->groups |= (1
>> << HID_GROUP_MULTITOUCH)? This change could be made regardless of the
>> parser logic.
>
> If nobody is against changing the values of the different groups across
> kernel version (which should be harmless), then I fully agree, we can
> use the group item as a bit field (but we would be able to only have 16
> different device groups).
Hmm, that might become a problem. E.g. all the HID sensors might be used
stand alone (without a sensor-hub, if someone modifies the drivers). But
I agree that currently the flags are just confusing and would introduce
them only if the number of groups reaches the limit.
>>> - hid->group = HID_GROUP_GENERIC;
>>> + parser = vzalloc(sizeof(struct hid_parser));
>>
>> Argh, I realize it is inevitable for this patch, but it still makes my
>> eyes bleed. The parser takes quite a bit of memory...
>
> Yep, my first attempt was to remove it, then I re-added it with a small
> tear...
So you actually create a new parser and the subject (that existing) of
this patch is misleading.
Regards,
Alexander Holler
^ permalink raw reply
* Re: kernel Oops: 0003 on usbhid_submit_report
From: Greg KH @ 2013-08-15 6:37 UTC (permalink / raw)
To: Jiri Kosina, Andreas Lillebø Holm; +Cc: linux-usb, linux-input
In-Reply-To: <16FA42AC8F654C3B81924591E607B342@sqhead.com>
On Wed, Aug 14, 2013 at 03:38:04PM +0200, Andreas Lillebø Holm wrote:
> Hi,
>
> On Tuesday, August 13, 2013 at 8:40 PM, Greg KH wrote:
> > > When communicating with AT90USB1287, at random intervals (1/25 boots)
> > > the linux hid_output_field Oopses and kills the communicating thread.
> > > The AT90USB1287 microcontroller uses LUFA library for usb/hid
> > > communication. It is trigged by a ioctl call from userspace and fails
> > > in a kernel paging request. The system is after the oops in a state
> > > where no hid commands is sent anymore and only a boot can fix the
> > > system.
> > >
> > > Keywords: usbhid hid
> > >
> > > Kernel version: Linux version 3.8.13-03081301-generic (apw@gomeisa) (gcc version 4.6.3 (Ubuntu/Linaro 4.6.3-1ubuntu5) ) #201305311535 SMP Fri May 31 19:44:30 UTC 2013
> > >
> > > Oopses:
> > > http://paste.debian.net/24305
> > > http://paste.debian.net/24306
> > > http://paste.debian.net/24307
> > >
> > > Code:
> > > The error is triggered by:
> > > ioctl(fd,HIDIOCSUSAGES, &ref_multi_u);
> > > ioctl(fd,HIDIOCSREPORT, &rep_info_u);
> > >
> > > Notes:
> > > It is very hard to reproduce so seems like race condition…
> > >
> > > Any tips to resolve/workaround this issue is appreciated and please
> > > let me know if my information is incomplete (This is my first kernel
> > > bug report)
> >
> >
> >
> >
> > Any chance you can try a supported kernel, like 3.10.6 or 3.11-rc5 to
> > see if that also causes problems? We can't do anything with
> > distro-specific kernel releases like your 3.8.13 release from Ubuntu,
> > sorry.
>
>
> I've now tried with kernel 3.10.6 (http://kernel.ubuntu.com/~kernel-ppa/mainline/v3.10.6-saucy/) and I can trigger the same issue on this kernel.
>
> Here is a paste from last Oops on this kernel:
> http://paste.debian.net/24993/
>
> I am also using usbmon to monitor the usb bus traffic, but cannot see anything that should cause the driver to Oops.
>
> Is there any way to find out what can trigger this issue?
Jiri should be the one to know this, Jiri, any ideas?
greg k-h
--
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 1/1] Input: htcpen - Fix incorrect placement of __initdata
From: Dmitry Torokhov @ 2013-08-15 6:51 UTC (permalink / raw)
To: Sachin Kamat; +Cc: linux-input@vger.kernel.org, Pau Oliva Fora
In-Reply-To: <CAK9yfHy-Q8W=0ujnnMmspsSJuwJeH-GMpOky+_GkPXf14mHK5w@mail.gmail.com>
Hi Sachin,
On Wed, Aug 14, 2013 at 11:55:22AM +0530, Sachin Kamat wrote:
> Hi Dmitry,
>
> On 7 August 2013 15:09, Sachin Kamat <sachin.kamat@linaro.org> wrote:
> > __initdata should be placed between the variable name and equal
> > sign for the variable to be placed in the intended section.
> >
> > Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
> > Cc: Pau Oliva Fora <pau@eslack.org>
> > ---
> > drivers/input/touchscreen/htcpen.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/touchscreen/htcpen.c b/drivers/input/touchscreen/htcpen.c
> > index 6c4fb84..6650085 100644
> > --- a/drivers/input/touchscreen/htcpen.c
> > +++ b/drivers/input/touchscreen/htcpen.c
> > @@ -221,7 +221,7 @@ static struct isa_driver htcpen_isa_driver = {
> > }
> > };
> >
> > -static struct dmi_system_id __initdata htcshift_dmi_table[] = {
> > +static struct dmi_system_id htcshift_dmi_table[] __initdata = {
> > {
> > .ident = "Shift",
> > .matches = {
> > --
> > 1.7.9.5
> >
>
> Thanks for applying my other patches. Looks like you missed this one though :)
Got it now, thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 28/29] drivers/input/{keyboard,serio}: simplify use of devm_ioremap_resource
From: Dmitry Torokhov @ 2013-08-15 7:09 UTC (permalink / raw)
To: Viresh Kumar
Cc: Julia Lawall, kernel-janitors, linux-input,
linux-kernel@vger.kernel.org, spear-devel
In-Reply-To: <CAOh2x=muvdm+tSR+ucUWLhb1vOvXB=_+vm_v9fHfa6mHUnhRRw@mail.gmail.com>
On Wed, Aug 14, 2013 at 04:51:02PM +0530, Viresh Kumar wrote:
> On Wed, Aug 14, 2013 at 4:33 PM, Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> > From: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > Remove unneeded error handling on the result of a call to
> > platform_get_resource when the value is passed to devm_ioremap_resource.
> >
> > Move the call to platform_get_resource adjacent to the call to
> > devm_ioremap_resource to make the connection between them more clear.
> >
> > A simplified version of the semantic patch that makes this change is as
> > follows: (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > expression pdev,res,n,e,e1;
> > expression ret != 0;
> > identifier l;
> > @@
> >
> > - res = platform_get_resource(pdev, IORESOURCE_MEM, n);
> > ... when != res
> > - if (res == NULL) { ... \(goto l;\|return ret;\) }
> > ... when != res
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, n);
> > e = devm_ioremap_resource(e1, res);
> > // </smpl>
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
> >
> > ---
>
> > drivers/input/keyboard/spear-keyboard.c | 7 +------
>
> For SPEAr:
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Thanks, applied.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: elantech - fix packet check for v3 and v4 hardware
From: Dmitry Torokhov @ 2013-08-15 7:12 UTC (permalink / raw)
To: Matteo Delfino; +Cc: linux-input
In-Reply-To: <1376473037-11211-1-git-send-email-kendatsuba@gmail.com>
Hi Matteo,
On Wed, Aug 14, 2013 at 11:37:17AM +0200, Matteo Delfino wrote:
> The signatures of v3 and v4 packets change depending on the value of a
> hardware flag called 'crc_enabled'. The packet type detection must change
> accordingly.
>
> This patch also restores a consistency check for v4 packets inadvertently
> removed by commit:
>
> 9eebed7de660c0b5ab129a9de4f89d20b60de68c
> Input: elantech - fix for newer hardware versions (v7)
>
> A note about the naming convention: v3 hardware is associated with IC body
> v5 while v4 hardware is associated with IC body v6 and v7. The above commit
> refers to IC body v7, not to v7 hardware.
>
> Tested on Samsung NP730U3E (fw = 0x675f05, ICv7, crc_enabled = 1)
>
> Tested-by: Giovanni Frigione <gio.frigione@gmail.com>
> Signed-off-by: Matteo Delfino <kendatsuba@gmail.com>
> ---
>
> Hello Dmitry,
> this patch is based on some information provided by elantech. It has
> been successfully tested on the above v4 hardware so the information
> seems to be reliable. More testing is needed on v3 hardware with
> crc_enabled=1 (if there's such hardware out there..). The behaviour
> should be unchanged for v3 and v4 hardware with crc_enabled=0 (the
> vast majority).
>
> I was asked to CC stable kernels too since this patch restores a
> consistency check on v4 packets removed by a previous commit already
> queued in linux-3.5.y-queue but, considering the above, I think it's
> best to leave this decision to you.
Once again, I think it should bake a bit in the trees before heading for
next, so I won't mark it for stable. Feel free to forward it later, once
it survives a few -rcs.
Thanks!
--
Dmitry
^ 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