* [PATCH 2/9] HID: multitouch: make sure the static list of class is not changed
From: Benjamin Tissoires @ 2018-05-29 9:57 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
Benjamin Tissoires
In-Reply-To: <20180529095800.13504-1-benjamin.tissoires@redhat.com>
const is a magic keyword here :)
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-multitouch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 38c4ca24343e..187d27f26eec 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -211,7 +211,7 @@ static int cypress_compute_slot(struct mt_device *td)
return -1;
}
-static struct mt_class mt_classes[] = {
+static const struct mt_class mt_classes[] = {
{ .name = MT_CLS_DEFAULT,
.quirks = MT_QUIRK_ALWAYS_VALID |
MT_QUIRK_CONTACT_CNT_ACCURATE },
@@ -1429,7 +1429,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
int ret, i;
struct mt_device *td;
- struct mt_class *mtclass = mt_classes; /* MT_CLS_DEFAULT */
+ const struct mt_class *mtclass = mt_classes; /* MT_CLS_DEFAULT */
for (i = 0; mt_classes[i].name ; i++) {
if (id->driver_data == mt_classes[i].name) {
--
2.14.3
^ permalink raw reply related
* [PATCH 1/9] Input: mt - export MT_TOOL in input_mt_init_slot()
From: Benjamin Tissoires @ 2018-05-29 9:57 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
Benjamin Tissoires
In-Reply-To: <20180529095800.13504-1-benjamin.tissoires@redhat.com>
Looks like we require users to set a tool but input_mt_init_slots() never
set the tool bit for us. Meaning that this is a useless information the
driver tries to forward.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/input/input-mt.c | 1 +
drivers/input/rmi4/rmi_2d_sensor.c | 2 --
drivers/input/touchscreen/atmel_mxt_ts.c | 2 --
drivers/input/touchscreen/hideep.c | 2 --
drivers/input/touchscreen/wacom_w8001.c | 2 --
5 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/input/input-mt.c b/drivers/input/input-mt.c
index a1bbec9cda8d..3900686c5d0f 100644
--- a/drivers/input/input-mt.c
+++ b/drivers/input/input-mt.c
@@ -57,6 +57,7 @@ int input_mt_init_slots(struct input_dev *dev, unsigned int num_slots,
mt->flags = flags;
input_set_abs_params(dev, ABS_MT_SLOT, 0, num_slots - 1, 0, 0);
input_set_abs_params(dev, ABS_MT_TRACKING_ID, 0, TRKID_MAX, 0, 0);
+ input_set_abs_params(dev, ABS_MT_TOOL_TYPE, 0, MT_TOOL_MAX, 0, 0);
if (flags & (INPUT_MT_POINTER | INPUT_MT_DIRECT)) {
__set_bit(EV_KEY, dev->evbit);
diff --git a/drivers/input/rmi4/rmi_2d_sensor.c b/drivers/input/rmi4/rmi_2d_sensor.c
index 8bb866c7b985..23e666bd2539 100644
--- a/drivers/input/rmi4/rmi_2d_sensor.c
+++ b/drivers/input/rmi4/rmi_2d_sensor.c
@@ -186,8 +186,6 @@ static void rmi_2d_sensor_set_input_params(struct rmi_2d_sensor *sensor)
input_set_abs_params(input, ABS_MT_TOUCH_MAJOR, 0, 0x0f, 0, 0);
input_set_abs_params(input, ABS_MT_TOUCH_MINOR, 0, 0x0f, 0, 0);
input_set_abs_params(input, ABS_MT_ORIENTATION, 0, 1, 0, 0);
- input_set_abs_params(input, ABS_MT_TOOL_TYPE,
- 0, MT_TOOL_MAX, 0, 0);
if (sensor->sensor_type == rmi_sensor_touchpad)
input_flags = INPUT_MT_POINTER;
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 54fe190fd4bc..0dcf48100dc1 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -2021,8 +2021,6 @@ static int mxt_initialize_input_device(struct mxt_data *data)
}
if (data->multitouch == MXT_TOUCH_MULTITOUCHSCREEN_T100) {
- input_set_abs_params(input_dev, ABS_MT_TOOL_TYPE,
- 0, MT_TOOL_MAX, 0, 0);
input_set_abs_params(input_dev, ABS_MT_DISTANCE,
MXT_DISTANCE_ACTIVE_TOUCH,
MXT_DISTANCE_HOVERING,
diff --git a/drivers/input/touchscreen/hideep.c b/drivers/input/touchscreen/hideep.c
index f1cd4dd9a4a3..980539d8d551 100644
--- a/drivers/input/touchscreen/hideep.c
+++ b/drivers/input/touchscreen/hideep.c
@@ -799,8 +799,6 @@ static int hideep_init_input(struct hideep_ts *ts)
input_set_capability(ts->input_dev, EV_ABS, ABS_MT_POSITION_Y);
input_set_abs_params(ts->input_dev, ABS_MT_PRESSURE, 0, 65535, 0, 0);
input_set_abs_params(ts->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
- input_set_abs_params(ts->input_dev, ABS_MT_TOOL_TYPE,
- 0, MT_TOOL_MAX, 0, 0);
touchscreen_parse_properties(ts->input_dev, true, &ts->prop);
if (ts->prop.max_x == 0 || ts->prop.max_y == 0) {
diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index 3715d1eace92..946dca30560b 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -529,8 +529,6 @@ static int w8001_setup_touch(struct w8001 *w8001, char *basename,
0, touch.x, 0, 0);
input_set_abs_params(dev, ABS_MT_POSITION_Y,
0, touch.y, 0, 0);
- input_set_abs_params(dev, ABS_MT_TOOL_TYPE,
- 0, MT_TOOL_MAX, 0, 0);
input_abs_set_res(dev, ABS_MT_POSITION_X, touch.panel_res);
input_abs_set_res(dev, ABS_MT_POSITION_Y, touch.panel_res);
--
2.14.3
^ permalink raw reply related
* [PATCH 0/9] Hid multitouch rewrite, support os system multi-axis devices
From: Benjamin Tissoires @ 2018-05-29 9:57 UTC (permalink / raw)
To: Jiri Kosina, Dmitry Torokhov
Cc: Mario.Limonciello, Peter Hutterer, linux-input, linux-kernel,
Benjamin Tissoires
Hi Jiri,
this heavy rewrite of hid-multitouch serves two main purposes:
- the first point is to enable the support of the Totem on the Dell Canvas 27.
This new type of devices (System Multi Axis) is used by Microsoft to show
a new circular menu that is used by the second hand while you interact with
your preferred hand with your tool.
I couldn't enable it before because hid-multitouch expected to have only one
multitouch collection, and this device is exported as a separate multitouch
collextion in the same HID device than the one containing the touch sensor.
- the second point is to streamline the process of the multitouch events. We
used to temporary store the events in a struct as they come in, and then do
the processing on the cache we just made. When doing the processing was also
not very clear. This made that even if hid-multitouch should IMO be merged in
hid-core, we couldn't.
The new processing of the events here adds a preparsing of the report in
one HID collection (application usage), and from now on, the processing
of the report is cleaner IMO. I still haven't merged hid-mt into hid-core,
because even if I wrote a bunch of unit tests trying to not break any devices,
we are not protected from a weird thing that magically happened before but is
now broken.
I must say that when I worked on the tests, I came to realise that some legacy
Win7 devices were better handled now. Initially I thought my new code broke them
but comparing the outputs from https://github.com/bentiss/hid-devices before
and after the changes, the new changes are closer to what I would expect by
looking at the raw HID events.
I also included 2 patches to enable the Surface Dial. It's a BLE device similar
to the Totem from Dell, except that you can buy it for roughly $80 instead of
$1800 for the Canvas.
Dmitry, 2 patches are of interest for you:
- 1/9 Input: mt - export MT_TOOL in input_mt_init_slot()
- 7/9 HID: input: enable Totem on the Dell Canvas 27
The second one exports a new MT_TOOL. In theory, I think patch 1/9 could be
carried through your tree but it'll be better to have the full series applied
at once.
Cheers,
Benjamin
Benjamin Tissoires (9):
Input: mt - export MT_TOOL in input_mt_init_slot()
HID: multitouch: make sure the static list of class is not changed
HID: multitouch: Store per collection multitouch data
HID: multitouch: store a per application quirks value
HID: multitouch: ditch mt_report_id
HID: multitouch: remove one copy of values
HID: input: enable Totem on the Dell Canvas 27
HID: core: do not upper bound the collection stack
HID: microsoft: support the Surface Dial
drivers/hid/hid-core.c | 17 +-
drivers/hid/hid-input.c | 3 +
drivers/hid/hid-microsoft.c | 49 +-
drivers/hid/hid-multitouch.c | 941 ++++++++++++++++++-------------
drivers/input/input-mt.c | 1 +
drivers/input/rmi4/rmi_2d_sensor.c | 2 -
drivers/input/touchscreen/atmel_mxt_ts.c | 2 -
drivers/input/touchscreen/hideep.c | 2 -
drivers/input/touchscreen/wacom_w8001.c | 2 -
include/linux/hid.h | 15 +-
include/uapi/linux/input.h | 3 +-
11 files changed, 616 insertions(+), 421 deletions(-)
--
2.14.3
^ permalink raw reply
* Re: [PATCH] HID: Add support for 146b:0902 Bigben Interactive Kids' gamepad
From: Hanno Zulla @ 2018-05-29 7:58 UTC (permalink / raw)
To: Roderick Colenbrander
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <729f18bc-65b7-9bfd-50ed-fbb1f168500d@hanno.de>
Hi,
> not fully adhere to the gamepad spec (e.g. xpad.c also doesn't map my> XBox-Controller's D-Pad to NORTH/EAST/SOUTH/WEST but instead to HAT0
in> the same way as this device does).
Sorry, that was wrong. I meant BTN_DPAD_* instead of HAT0, despite them
being digital buttons.
Kind regards,
Hanno
^ permalink raw reply
* Re: [PATCH] HID: Add support for 146b:0902 Bigben Interactive Kids' gamepad
From: Hanno Zulla @ 2018-05-29 7:46 UTC (permalink / raw)
To: Roderick Colenbrander
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <CAEc3jaB7jPJ_xa5rFrunwcPgkG9VrCdJSbb=BrQsHKxrO9AixQ@mail.gmail.com>
Hi Roderick,
Hi Benjamin,
thanks for the review!
> In terms of code style, I noticed a lot of C++ style comments which> are not part of the kernel code style instead use C comments. To
check> for potential other issues as well, run your patch through>
'scripts/checkpatch.pl'.
Okay, will check with that.
> I noticed you fixed up reported descriptors a bit to get axes remapped
> in a different way. This is reasonable as the default mappings are
> often not good. However, I would suggest use the mapping functions
> instead (e.g. see hid-sony.c and other drivers). It also allows you to
> properly remap buttons as well. I suspect the current button mapping
> is all over the place as well. Make sure axis and button mapping
> complies to the Linux gamepad spec (see
> Documentation/input/gamepad.rst).
The main reason for the fixup was to cleanup the output part of the
descriptor, as the device's original descriptor seemed to use
nonsensical/undefined "Usage" values.
Those two axis remappings were the only two necessary. The rest of the
mappings are fine and look okay when compared to other drivers that do
not fully adhere to the gamepad spec (e.g. xpad.c also doesn't map my
XBox-Controller's D-Pad to NORTH/EAST/SOUTH/WEST but instead to HAT0 in
the same way as this device does).
I had assumed that using a descriptor fixup and leaving the actual work
to the standard driver was cleaner than meddling with the incoming data
in a mapping function.
Actually, I was researching if I could fix the descriptor so that no
LED/FF-specific code is necessary, but my knowledge of HID was too
limited to achieve that.
> In addition to those comments, please also try to follow the
> submission guidelines
Ah, thank you.
> Also, keep point 9 in mind ("Don't get discouraged - or impatient").
Nah. Don't worry! :-D
Thanks & kind regards,
Hanno
^ permalink raw reply
* Re: [PATCH] HID: Add support for 146b:0902 Bigben Interactive Kids' gamepad
From: Benjamin Tissoires @ 2018-05-29 7:12 UTC (permalink / raw)
To: Roderick Colenbrander; +Cc: Hanno Zulla, Jiri Kosina, linux-input, lkml
In-Reply-To: <CAEc3jaB7jPJ_xa5rFrunwcPgkG9VrCdJSbb=BrQsHKxrO9AixQ@mail.gmail.com>
On Tue, May 29, 2018 at 12:57 AM, Roderick Colenbrander
<thunderbird2k@gmail.com> wrote:
> Hi Hanno,
>
> A few suggestions for your patch based on a quick review (had limited time).
>
> In terms of code style, I noticed a lot of C++ style comments which
> are not part of the kernel code style instead use C comments. To check
> for potential other issues as well, run your patch through
> 'scripts/checkpatch.pl'.
>
> I noticed you fixed up reported descriptors a bit to get axes remapped
> in a different way. This is reasonable as the default mappings are
> often not good. However, I would suggest use the mapping functions
> instead (e.g. see hid-sony.c and other drivers). It also allows you to
> properly remap buttons as well. I suspect the current button mapping
> is all over the place as well. Make sure axis and button mapping
> complies to the Linux gamepad spec (see
> Documentation/input/gamepad.rst).
Thanks a lot Roderick for the review.
In addition to those comments, please also try to follow the
submission guidelines (point 6 of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst).
It's easier for us to share comments if your patch is inlined in the
email, so we can directly pinpoint which line is in trouble.
Also, keep point 9 in mind ("Don't get discouraged - or impatient").
For the first submissions, it's usual to have several revisions of the
same series.
Cheers,
Benjamin
>
> Thanks,
> Roderick
>
> On Wed, May 23, 2018 at 8:57 AM, Hanno Zulla <abos@hanno.de> wrote:
>> Hello,
>>
>> this is a driver for the "BigBen Interactive Kid-friendly Wired
>> Controller PS3OFMINIPAD SONY" [1] with USB id 146b:0902. It was
>> originally sold as a PS3 accessory and serves as a very nice gamepad for
>> Retropie.
>>
>> With the help of serialusb [2], it was possible to analyze the protocol
>> used by the gamepad and fix the missing entries for the descriptor.
>>
>>
>> This is my *first* driver, so please review accordingly.
>>
>>
>> Three things where I'm not sure and hope to hear from you about:
>>
>> - is that the correct use of hid_validate_values() in bigben_probe()?
>>
>> - is the error handling in bigben_probe() correct?
>>
>> - how do I properly test possible suspend/resume scenarios?
>>
>> Thanks,
>>
>> Hanno
>>
>>
>> [1]
>> <https://www.bigben-interactive.co.uk/bigben-products/gaming-accessories/wired-mini-controller-ps3>
>>
>> [2] <https://github.com/matlo/serialusb>
^ permalink raw reply
* Re: [PATCH] HID: Add support for 146b:0902 Bigben Interactive Kids' gamepad
From: Roderick Colenbrander @ 2018-05-28 22:57 UTC (permalink / raw)
To: Hanno Zulla; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <31245206-038e-4a5b-a1a1-8877463556a7@hanno.de>
Hi Hanno,
A few suggestions for your patch based on a quick review (had limited time).
In terms of code style, I noticed a lot of C++ style comments which
are not part of the kernel code style instead use C comments. To check
for potential other issues as well, run your patch through
'scripts/checkpatch.pl'.
I noticed you fixed up reported descriptors a bit to get axes remapped
in a different way. This is reasonable as the default mappings are
often not good. However, I would suggest use the mapping functions
instead (e.g. see hid-sony.c and other drivers). It also allows you to
properly remap buttons as well. I suspect the current button mapping
is all over the place as well. Make sure axis and button mapping
complies to the Linux gamepad spec (see
Documentation/input/gamepad.rst).
Thanks,
Roderick
On Wed, May 23, 2018 at 8:57 AM, Hanno Zulla <abos@hanno.de> wrote:
> Hello,
>
> this is a driver for the "BigBen Interactive Kid-friendly Wired
> Controller PS3OFMINIPAD SONY" [1] with USB id 146b:0902. It was
> originally sold as a PS3 accessory and serves as a very nice gamepad for
> Retropie.
>
> With the help of serialusb [2], it was possible to analyze the protocol
> used by the gamepad and fix the missing entries for the descriptor.
>
>
> This is my *first* driver, so please review accordingly.
>
>
> Three things where I'm not sure and hope to hear from you about:
>
> - is that the correct use of hid_validate_values() in bigben_probe()?
>
> - is the error handling in bigben_probe() correct?
>
> - how do I properly test possible suspend/resume scenarios?
>
> Thanks,
>
> Hanno
>
>
> [1]
> <https://www.bigben-interactive.co.uk/bigben-products/gaming-accessories/wired-mini-controller-ps3>
>
> [2] <https://github.com/matlo/serialusb>
^ permalink raw reply
* Re: [PATCH] HID: rmi: use HID_QUIRK_NO_INPUT_SYNC
From: Benjamin Tissoires @ 2018-05-28 16:38 UTC (permalink / raw)
To: Jiri Kosina, Andrew Duggan
Cc: Oscar Morante, Peter Hutterer, open list:HID CORE LAYER, lkml,
Benjamin Tissoires
In-Reply-To: <20180525125106.18008-1-benjamin.tissoires@redhat.com>
On Fri, May 25, 2018 at 2:51 PM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> When we receive a RMI4 report, we should not unconditionally send an
> input_sync event. Instead, we should let the rmi4 transport layer do it
> for us.
>
> This fixes a situation where we might receive X in a report and the rest
> in a subsequent one. And this messes up user space.
>
> Link: https://bugs.freedesktop.org/show_bug.cgi?id=100436
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
>
> Hi,
>
> Oscar, do you mind if we add your "Tested-by: Oscar Morante <your@email>"?
As mentioned in comment 25 of the bug above
(https://bugs.freedesktop.org/show_bug.cgi?id=100436#c25), oscar is
fine adding his tested-by line.
Cheers,
Benjamin
>
> Andrew, can you check for any sides effects please?
>
> Cheers,
> Benjamin
>
> drivers/hid/hid-rmi.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
> index 9c9362149641..9e33165250a3 100644
> --- a/drivers/hid/hid-rmi.c
> +++ b/drivers/hid/hid-rmi.c
> @@ -413,6 +413,24 @@ static int rmi_event(struct hid_device *hdev, struct hid_field *field,
> return 0;
> }
>
> +static void rmi_report(struct hid_device *hid, struct hid_report *report)
> +{
> + struct hid_field *field = report->field[0];
> +
> + if (!(hid->claimed & HID_CLAIMED_INPUT))
> + return;
> +
> + switch (report->id) {
> + case RMI_READ_DATA_REPORT_ID:
> + /* fall-through */
> + case RMI_ATTN_REPORT_ID:
> + return;
> + }
> +
> + if (field && field->hidinput && field->hidinput->input)
> + input_sync(field->hidinput->input);
> +}
> +
> #ifdef CONFIG_PM
> static int rmi_suspend(struct hid_device *hdev, pm_message_t message)
> {
> @@ -637,6 +655,7 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
> hid_set_drvdata(hdev, data);
>
> hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
> + hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC;
>
> ret = hid_parse(hdev);
> if (ret) {
> @@ -744,6 +763,7 @@ static struct hid_driver rmi_driver = {
> .remove = rmi_remove,
> .event = rmi_event,
> .raw_event = rmi_raw_event,
> + .report = rmi_report,
> .input_mapping = rmi_input_mapping,
> .input_configured = rmi_input_configured,
> #ifdef CONFIG_PM
> --
> 2.14.3
>
^ permalink raw reply
* Re: [PATCH] mfd: intel-lpss: Fix Intel Cannon Lake LPSS I2C input clock
From: Andy Shevchenko @ 2018-05-28 15:36 UTC (permalink / raw)
To: Jarkko Nikula, linux-kernel
Cc: Lee Jones, Mika Westerberg, linux-i2c, linux-input, Jian-Hong Pan,
Chris Chiu, Daniel Drake, stable
In-Reply-To: <20180518083827.20626-1-jarkko.nikula@linux.intel.com>
On Fri, 2018-05-18 at 11:38 +0300, Jarkko Nikula wrote:
> Intel Cannon Lake PCH has much higher 216 MHz input clock to LPSS I2C
> than Sunrisepoint which uses 120 MHz. Preliminary information was that
> both share the same clock rate but actual silicon implements elevated
> rate for better support for 3.4 MHz high-speed I2C.
>
> This incorrect input clock rate results too high I2C bus clock in case
> ACPI doesn't provide tuned I2C timing parameters since I2C host
> controller driver calculates them from input clock rate.
>
> Fix this by using the correct rate. We still share the same 230 ns SDA
> hold time value than Sunrisepoint.
>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
P.S. Documentation we have is not perfect, that's why previously I
though about Broxton/Cannonlake case as single, which is wrong, they are
different in terms of i2c clock organization.
> Cc: stable@vger.kernel.org
> Fixes: b418bbff36dd ("mfd: intel-lpss: Add Intel Cannonlake PCI IDs")
> Reported-by: Jian-Hong Pan <jian-hong@endlessm.com>
> Reported-by: Chris Chiu <chiu@endlessm.com>
> Reported-by: Daniel Drake <drake@endlessm.com>
> Signed-off-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> ---
> Hi Jian-Hong, Chris and Daniel. Could you test does this fix your
> touchpad issue?
> ---
> drivers/mfd/intel-lpss-pci.c | 25 +++++++++++++++----------
> 1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-
> pci.c
> index d1c46de89eb4..d9ae983095c5 100644
> --- a/drivers/mfd/intel-lpss-pci.c
> +++ b/drivers/mfd/intel-lpss-pci.c
> @@ -124,6 +124,11 @@ static const struct intel_lpss_platform_info
> apl_i2c_info = {
> .properties = apl_i2c_properties,
> };
>
> +static const struct intel_lpss_platform_info cnl_i2c_info = {
> + .clk_rate = 216000000,
> + .properties = spt_i2c_properties,
> +};
> +
> static const struct pci_device_id intel_lpss_pci_ids[] = {
> /* BXT A-Step */
> { PCI_VDEVICE(INTEL, 0x0aac), (kernel_ulong_t)&bxt_i2c_info
> },
> @@ -207,13 +212,13 @@ static const struct pci_device_id
> intel_lpss_pci_ids[] = {
> { PCI_VDEVICE(INTEL, 0x9daa), (kernel_ulong_t)&spt_info },
> { PCI_VDEVICE(INTEL, 0x9dab), (kernel_ulong_t)&spt_info },
> { PCI_VDEVICE(INTEL, 0x9dfb), (kernel_ulong_t)&spt_info },
> - { PCI_VDEVICE(INTEL, 0x9dc5), (kernel_ulong_t)&spt_i2c_info
> },
> - { PCI_VDEVICE(INTEL, 0x9dc6), (kernel_ulong_t)&spt_i2c_info
> },
> + { PCI_VDEVICE(INTEL, 0x9dc5), (kernel_ulong_t)&cnl_i2c_info
> },
> + { PCI_VDEVICE(INTEL, 0x9dc6), (kernel_ulong_t)&cnl_i2c_info
> },
> { PCI_VDEVICE(INTEL, 0x9dc7), (kernel_ulong_t)&spt_uart_info
> },
> - { PCI_VDEVICE(INTEL, 0x9de8), (kernel_ulong_t)&spt_i2c_info
> },
> - { PCI_VDEVICE(INTEL, 0x9de9), (kernel_ulong_t)&spt_i2c_info
> },
> - { PCI_VDEVICE(INTEL, 0x9dea), (kernel_ulong_t)&spt_i2c_info
> },
> - { PCI_VDEVICE(INTEL, 0x9deb), (kernel_ulong_t)&spt_i2c_info
> },
> + { PCI_VDEVICE(INTEL, 0x9de8), (kernel_ulong_t)&cnl_i2c_info
> },
> + { PCI_VDEVICE(INTEL, 0x9de9), (kernel_ulong_t)&cnl_i2c_info
> },
> + { PCI_VDEVICE(INTEL, 0x9dea), (kernel_ulong_t)&cnl_i2c_info
> },
> + { PCI_VDEVICE(INTEL, 0x9deb), (kernel_ulong_t)&cnl_i2c_info
> },
> /* SPT-H */
> { PCI_VDEVICE(INTEL, 0xa127), (kernel_ulong_t)&spt_uart_info
> },
> { PCI_VDEVICE(INTEL, 0xa128), (kernel_ulong_t)&spt_uart_info
> },
> @@ -240,10 +245,10 @@ static const struct pci_device_id
> intel_lpss_pci_ids[] = {
> { PCI_VDEVICE(INTEL, 0xa32b), (kernel_ulong_t)&spt_info },
> { PCI_VDEVICE(INTEL, 0xa37b), (kernel_ulong_t)&spt_info },
> { PCI_VDEVICE(INTEL, 0xa347), (kernel_ulong_t)&spt_uart_info
> },
> - { PCI_VDEVICE(INTEL, 0xa368), (kernel_ulong_t)&spt_i2c_info
> },
> - { PCI_VDEVICE(INTEL, 0xa369), (kernel_ulong_t)&spt_i2c_info
> },
> - { PCI_VDEVICE(INTEL, 0xa36a), (kernel_ulong_t)&spt_i2c_info
> },
> - { PCI_VDEVICE(INTEL, 0xa36b), (kernel_ulong_t)&spt_i2c_info
> },
> + { PCI_VDEVICE(INTEL, 0xa368), (kernel_ulong_t)&cnl_i2c_info
> },
> + { PCI_VDEVICE(INTEL, 0xa369), (kernel_ulong_t)&cnl_i2c_info
> },
> + { PCI_VDEVICE(INTEL, 0xa36a), (kernel_ulong_t)&cnl_i2c_info
> },
> + { PCI_VDEVICE(INTEL, 0xa36b), (kernel_ulong_t)&cnl_i2c_info
> },
> { }
> };
> MODULE_DEVICE_TABLE(pci, intel_lpss_pci_ids);
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* [PATCH] Input: elantech - Fix V4 report decoding for module with middle key
From: KT Liao @ 2018-05-28 11:33 UTC (permalink / raw)
To: linux-kernel, linux-input, dmitry.torokhov, ulrik.debie-os
Cc: phoenix, kt.liao, aaron.ma, josh.chen
Some touchpad has middle key and it will be indicated in bit 2 of packet[0].
We need to fix V4 formation's byte mask to prevent error decoding.
Signed-off-by: KT Liao <kt.liao@emc.com.tw>
---
drivers/input/mouse/elantech.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index fb4d902..f39dc66 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -799,7 +799,7 @@ static int elantech_packet_check_v4(struct psmouse *psmouse)
else if (ic_version == 7 && etd->info.samples[1] == 0x2A)
sanity_check = ((packet[3] & 0x1c) == 0x10);
else
- sanity_check = ((packet[0] & 0x0c) == 0x04 &&
+ sanity_check = ((packet[0] & 0x08) == 0x00 &&
(packet[3] & 0x1c) == 0x10);
if (!sanity_check)
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] HID: rmi: use HID_QUIRK_NO_INPUT_SYNC
From: Peter Hutterer @ 2018-05-27 23:12 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Andrew Duggan, Oscar Morante, linux-input,
linux-kernel
In-Reply-To: <20180525125106.18008-1-benjamin.tissoires@redhat.com>
On Fri, May 25, 2018 at 02:51:06PM +0200, Benjamin Tissoires wrote:
> When we receive a RMI4 report, we should not unconditionally send an
> input_sync event. Instead, we should let the rmi4 transport layer do it
> for us.
>
> This fixes a situation where we might receive X in a report and the rest
> in a subsequent one. And this messes up user space.
>
> Link: https://bugs.freedesktop.org/show_bug.cgi?id=100436
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
yes please!
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
Cheers,
Peter
>
> Hi,
>
> Oscar, do you mind if we add your "Tested-by: Oscar Morante <your@email>"?
>
> Andrew, can you check for any sides effects please?
>
> Cheers,
> Benjamin
>
> drivers/hid/hid-rmi.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
> index 9c9362149641..9e33165250a3 100644
> --- a/drivers/hid/hid-rmi.c
> +++ b/drivers/hid/hid-rmi.c
> @@ -413,6 +413,24 @@ static int rmi_event(struct hid_device *hdev, struct hid_field *field,
> return 0;
> }
>
> +static void rmi_report(struct hid_device *hid, struct hid_report *report)
> +{
> + struct hid_field *field = report->field[0];
> +
> + if (!(hid->claimed & HID_CLAIMED_INPUT))
> + return;
> +
> + switch (report->id) {
> + case RMI_READ_DATA_REPORT_ID:
> + /* fall-through */
> + case RMI_ATTN_REPORT_ID:
> + return;
> + }
> +
> + if (field && field->hidinput && field->hidinput->input)
> + input_sync(field->hidinput->input);
> +}
> +
> #ifdef CONFIG_PM
> static int rmi_suspend(struct hid_device *hdev, pm_message_t message)
> {
> @@ -637,6 +655,7 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
> hid_set_drvdata(hdev, data);
>
> hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
> + hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC;
>
> ret = hid_parse(hdev);
> if (ret) {
> @@ -744,6 +763,7 @@ static struct hid_driver rmi_driver = {
> .remove = rmi_remove,
> .event = rmi_event,
> .raw_event = rmi_raw_event,
> + .report = rmi_report,
> .input_mapping = rmi_input_mapping,
> .input_configured = rmi_input_configured,
> #ifdef CONFIG_PM
> --
> 2.14.3
>
^ permalink raw reply
* [PATCH V2] cros_ec_keyb: Mark cros_ec_keyb driver as wake enabled device.
From: Ravi Chandra Sadineni @ 2018-05-26 1:14 UTC (permalink / raw)
To: dmitry.torokhov, lee.jones, ravisadineni, ravisadineni, dtor,
briannorris
Cc: tbroch, linux-kernel, linux-input, rajatja, bleung
In-Reply-To: <20180524234213.GE177107@dtor-ws>
Mark cros_ec_keyb has wake enabled by default. If we see a MKBP event
related to keyboard, call pm_wakeup_event() to make sure wakeup
triggers are accounted to keyb during suspend resume path.
Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
---
V2: Marked the ckdev as wake enabled instead of input devices.
drivers/input/keyboard/cros_ec_keyb.c | 21 +++++++++++++++++----
drivers/mfd/cros_ec.c | 19 +++++++------------
2 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
index 79eb29550c348..a7c96f0317123 100644
--- a/drivers/input/keyboard/cros_ec_keyb.c
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -245,12 +245,17 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
switch (ckdev->ec->event_data.event_type) {
case EC_MKBP_EVENT_KEY_MATRIX:
/*
- * If EC is not the wake source, discard key state changes
+ * If Keyb is not wake enabled, discard key state changes
* during suspend.
*/
- if (queued_during_suspend)
+ if (queued_during_suspend
+ && !device_may_wakeup(ckdev->dev))
return NOTIFY_OK;
+ if (device_may_wakeup(ckdev->dev))
+ pm_wakeup_event(ckdev->dev, 0);
+
+
if (ckdev->ec->event_size != ckdev->cols) {
dev_err(ckdev->dev,
"Discarded incomplete key matrix event.\n");
@@ -265,18 +270,25 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
val = get_unaligned_le32(&ckdev->ec->event_data.data.sysrq);
dev_dbg(ckdev->dev, "sysrq code from EC: %#x\n", val);
handle_sysrq(val);
+
+ if (device_may_wakeup(ckdev->dev))
+ pm_wakeup_event(ckdev->dev, 0);
break;
case EC_MKBP_EVENT_BUTTON:
case EC_MKBP_EVENT_SWITCH:
/*
- * If EC is not the wake source, discard key state
+ * If keyb is not wake enabled, discard key state
* changes during suspend. Switches will be re-checked in
* cros_ec_keyb_resume() to be sure nothing is lost.
*/
- if (queued_during_suspend)
+ if (queued_during_suspend
+ && !device_may_wakeup(ckdev->dev))
return NOTIFY_OK;
+ if (device_may_wakeup(ckdev->dev))
+ pm_wakeup_event(ckdev->dev, 0);
+
if (ckdev->ec->event_data.event_type == EC_MKBP_EVENT_BUTTON) {
val = get_unaligned_le32(
&ckdev->ec->event_data.data.buttons);
@@ -639,6 +651,7 @@ static int cros_ec_keyb_probe(struct platform_device *pdev)
return err;
}
+ device_init_wakeup(ckdev->dev, true);
return 0;
}
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index d61024141e2b6..36156a41499c9 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -229,7 +229,7 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev)
}
EXPORT_SYMBOL(cros_ec_suspend);
-static void cros_ec_drain_events(struct cros_ec_device *ec_dev)
+static void cros_ec_report_events_during_suspend(struct cros_ec_device *ec_dev)
{
while (cros_ec_get_next_event(ec_dev, NULL) > 0)
blocking_notifier_call_chain(&ec_dev->event_notifier,
@@ -253,21 +253,16 @@ int cros_ec_resume(struct cros_ec_device *ec_dev)
dev_dbg(ec_dev->dev, "Error %d sending resume event to ec",
ret);
- /*
- * In some cases, we need to distinguish between events that occur
- * during suspend if the EC is not a wake source. For example,
- * keypresses during suspend should be discarded if it does not wake
- * the system.
- *
- * If the EC is not a wake source, drain the event queue and mark them
- * as "queued during suspend".
- */
if (ec_dev->wake_enabled) {
disable_irq_wake(ec_dev->irq);
ec_dev->wake_enabled = 0;
- } else {
- cros_ec_drain_events(ec_dev);
}
+ /*
+ * Let the mfd devices know about events that occur during
+ * suspend. This way the clients know what to do with them.
+ */
+ cros_ec_report_events_during_suspend(ec_dev);
+
return 0;
}
--
2.17.0.921.gf22659ad46-goog
^ permalink raw reply related
* Re: [PATCH] HID: steam: select CONFIG_POWER_SUPPLY
From: Benjamin Tissoires @ 2018-05-25 16:40 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jiri Kosina, Rodrigo Rivas Costa, open list:HID CORE LAYER, lkml
In-Reply-To: <20180525153118.3015806-1-arnd@arndb.de>
On Fri, May 25, 2018 at 5:30 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> Using the power supply APIs requires selecting the appropriate
> Kconfig symbol, otherwise we get this build failure:
>
> drivers/hid/hid-steam.o: In function `steam_unregister':
> hid-steam.c:(.text+0x1cc): undefined reference to `power_supply_unregister'
> drivers/hid/hid-steam.o: In function `steam_battery_get_property':
> hid-steam.c:(.text+0x2d2): undefined reference to `power_supply_get_drvdata'
> drivers/hid/hid-steam.o: In function `steam_raw_event':
> hid-steam.c:(.text+0xcba): undefined reference to `power_supply_changed'
> drivers/hid/hid-steam.o: In function `steam_register':
> hid-steam.c:(.text+0x13e3): undefined reference to `power_supply_register'
> hid-steam.c:(.text+0x13fe): undefined reference to `power_supply_powers'
>
> Fixes: f82719790751 ("HID: steam: add battery device.")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Looks good:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> ---
> drivers/hid/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 8a865e3ccea6..a49a10437c40 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -848,6 +848,7 @@ config HID_SPEEDLINK
> config HID_STEAM
> tristate "Steam Controller support"
> depends on HID
> + select POWER_SUPPLY
> ---help---
> Say Y here if you have a Steam Controller if you want to use it
> without running the Steam Client. It supports both the wired and
> --
> 2.9.0
>
^ permalink raw reply
* Re: [PATCH v2 1/3] input: touchscreen: edt-ft5x06: don't make device a wakeup source by default
From: Rob Herring @ 2018-05-25 15:52 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Mark Rutland, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Sascha Hauer, linux-input, Fabio Estevam, Shawn Guo, Daniel Mack
In-Reply-To: <20180524231732.GC177107@dtor-ws>
On Thu, May 24, 2018 at 6:17 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, May 23, 2018 at 09:45:05AM -0500, Rob Herring wrote:
>> On Wed, May 23, 2018 at 3:27 AM, Daniel Mack <daniel@zonque.org> wrote:
>> > On Tuesday, May 22, 2018 07:54 PM, Rob Herring wrote:
>> >>
>> >> On Thu, May 17, 2018 at 11:05:50AM +0200, Daniel Mack wrote:
>> >>>
>> >>> Allow configuring the device as wakeup source through device properties,
>> >>> as
>> >>> not all platforms want to wake up on touch screen activity.
>> >>>
>> >>> The I2C core automatically reads the "wakeup-source" DT property to
>> >>> configure a device's wakeup capability, and board supports files can set
>> >>> I2C_CLIENT_WAKE in the flags.
>> >>
>> >>
>> >> This will break wake-up on working systems. Looks like mostly i.MX, but
>> >> there's one AM437x board. If that board doesn't care, then it is up to
>> >> Shawn.
>> >
>> >
>> > I added the property to the dts files, but as Dmitry pointed out, I missed
>> > some. Sorry for that.
>>
>> Just adding the property to dts files doesn't fix the compatibility
>> problem. If a user uses an existing dtb (before this change) with a
>> new kernel (after this change), then wakeup will stop working.
>
> Is this a practical problem though? Do we know of any products with
> this touch panel that use DTS not distributed with the kernel?
Distribution of dts with kernel is irrelevant. It is how the dtb's are
handled that matters. You still need to ask in tree users.
I can't have any way of knowing what DTs may exist as there is only
one upstream repository of dts files. There's not really a good
solution on this to avoid breaking users, so we'll just have to see if
anyone (besides the known users) complains.
Rob
^ permalink raw reply
* [PATCH] HID: steam: select CONFIG_POWER_SUPPLY
From: Arnd Bergmann @ 2018-05-25 15:30 UTC (permalink / raw)
To: Jiri Kosina
Cc: Arnd Bergmann, Benjamin Tissoires, Rodrigo Rivas Costa,
linux-input, linux-kernel
Using the power supply APIs requires selecting the appropriate
Kconfig symbol, otherwise we get this build failure:
drivers/hid/hid-steam.o: In function `steam_unregister':
hid-steam.c:(.text+0x1cc): undefined reference to `power_supply_unregister'
drivers/hid/hid-steam.o: In function `steam_battery_get_property':
hid-steam.c:(.text+0x2d2): undefined reference to `power_supply_get_drvdata'
drivers/hid/hid-steam.o: In function `steam_raw_event':
hid-steam.c:(.text+0xcba): undefined reference to `power_supply_changed'
drivers/hid/hid-steam.o: In function `steam_register':
hid-steam.c:(.text+0x13e3): undefined reference to `power_supply_register'
hid-steam.c:(.text+0x13fe): undefined reference to `power_supply_powers'
Fixes: f82719790751 ("HID: steam: add battery device.")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/hid/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 8a865e3ccea6..a49a10437c40 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -848,6 +848,7 @@ config HID_SPEEDLINK
config HID_STEAM
tristate "Steam Controller support"
depends on HID
+ select POWER_SUPPLY
---help---
Say Y here if you have a Steam Controller if you want to use it
without running the Steam Client. It supports both the wired and
--
2.9.0
^ permalink raw reply related
* [PATCH] HID: rmi: use HID_QUIRK_NO_INPUT_SYNC
From: Benjamin Tissoires @ 2018-05-25 12:51 UTC (permalink / raw)
To: Jiri Kosina, Andrew Duggan
Cc: Oscar Morante, Peter Hutterer, linux-input, linux-kernel,
Benjamin Tissoires
When we receive a RMI4 report, we should not unconditionally send an
input_sync event. Instead, we should let the rmi4 transport layer do it
for us.
This fixes a situation where we might receive X in a report and the rest
in a subsequent one. And this messes up user space.
Link: https://bugs.freedesktop.org/show_bug.cgi?id=100436
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
Hi,
Oscar, do you mind if we add your "Tested-by: Oscar Morante <your@email>"?
Andrew, can you check for any sides effects please?
Cheers,
Benjamin
drivers/hid/hid-rmi.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 9c9362149641..9e33165250a3 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -413,6 +413,24 @@ static int rmi_event(struct hid_device *hdev, struct hid_field *field,
return 0;
}
+static void rmi_report(struct hid_device *hid, struct hid_report *report)
+{
+ struct hid_field *field = report->field[0];
+
+ if (!(hid->claimed & HID_CLAIMED_INPUT))
+ return;
+
+ switch (report->id) {
+ case RMI_READ_DATA_REPORT_ID:
+ /* fall-through */
+ case RMI_ATTN_REPORT_ID:
+ return;
+ }
+
+ if (field && field->hidinput && field->hidinput->input)
+ input_sync(field->hidinput->input);
+}
+
#ifdef CONFIG_PM
static int rmi_suspend(struct hid_device *hdev, pm_message_t message)
{
@@ -637,6 +655,7 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
hid_set_drvdata(hdev, data);
hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
+ hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC;
ret = hid_parse(hdev);
if (ret) {
@@ -744,6 +763,7 @@ static struct hid_driver rmi_driver = {
.remove = rmi_remove,
.event = rmi_event,
.raw_event = rmi_raw_event,
+ .report = rmi_report,
.input_mapping = rmi_input_mapping,
.input_configured = rmi_input_configured,
#ifdef CONFIG_PM
--
2.14.3
^ permalink raw reply related
* Re: [PATCH v4 1/2] dt-bindings: input: Add Add Spreadtrum SC27xx vibrator documentation
From: Baolin Wang @ 2018-05-25 8:54 UTC (permalink / raw)
To: Rob Herring
Cc: Dmitry Torokhov, Mark Rutland, Orson Zhai, Chunyan Zhang,
open list:HID CORE LAYER, DTML, LKML, xiaotong.lu
In-Reply-To: <20180518165408.GA31804@rob-hp-laptop>
On 19 May 2018 at 00:54, Rob Herring <robh@kernel.org> wrote:
> On Thu, May 10, 2018 at 01:08:33PM +0800, Baolin Wang wrote:
>> From: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
>>
>> This patch adds the binding documentation for Spreadtrum SC27xx series
>> vibrator device.
>>
>> Signed-off-by: Xiaotong Lu <xiaotong.lu@spreadtrum.com>
>> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>
>> ---
>> Changes since v3:
>> - Change compatible string to explicit Soc name.
>> - Add parent MFD node.
>>
>> Changes since v2:
>> - No updates.
>>
>> Changes since v1:
>> - No updates.
>> ---
>> .../bindings/input/sprd,sc27xx-vibra.txt | 23 ++++++++++++++++++++
>> 1 file changed, 23 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/sprd,sc27xx-vibra.txt
>
> Reviewed-by: Rob Herring <robh@kernel.org>
Thanks Rob.
Hi Dmitry,
Could you apply this patch set if there are no other issues? Thanks.
--
Baolin.wang
Best Regards
^ permalink raw reply
* Re: [PATCH] cros_ec_keyb: Increment the wakeup count to the specific mfd device.
From: Dmitry Torokhov @ 2018-05-24 23:42 UTC (permalink / raw)
To: Ravi Chandra Sadineni
Cc: lee.jones, ravisadineni, dtor, tbroch, linux-kernel, linux-input,
rajatja, bleung
In-Reply-To: <20180523182958.25195-1-ravisadineni@chromium.org>
Hi Ravi,
On Wed, May 23, 2018 at 11:29:58AM -0700, Ravi Chandra Sadineni wrote:
> If the IRQ is processed during resume, increment the wakeup count to the
> specific mfd device based on the event, if the mfd device is wake enabled.
> This helps in identifying the specific device that caused the wake.
>
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
> drivers/input/keyboard/cros_ec_keyb.c | 20 +++++++++++++++-----
> drivers/mfd/cros_ec.c | 25 +++++++++++--------------
> 2 files changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> index 79eb29550c348..9b39289774405 100644
> --- a/drivers/input/keyboard/cros_ec_keyb.c
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -245,12 +245,16 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
> switch (ckdev->ec->event_data.event_type) {
> case EC_MKBP_EVENT_KEY_MATRIX:
> /*
> - * If EC is not the wake source, discard key state changes
> - * during suspend.
> + * If keyb input device is not wake enabled, discard key
> + * state changes during suspend.
> */
> - if (queued_during_suspend)
> + if (queued_during_suspend && ckdev->idev
> + && !device_may_wakeup(&ckdev->idev->dev))
> return NOTIFY_OK;
>
> + else if (queued_during_suspend && ckdev->idev)
> + pm_wakeup_event(&ckdev->idev->dev, 0);
Are we reporting wakeup event on the right device? Normally we only
report wakeups on hardware device whereas here we using input devices
which are logical abstractions...
In any case this can be collapsed a bit:
if (queued_during_suspend && ckdev->idev) {
if (!device_may_wakeup(&ckdev->idev->dev)
return NOTIFY_OK;
pm_wakeup_event(&ckdev->idev->dev, 0);
}
But I think you should actually look at ckdev->dev device here.
Also, why do we ignore EC_MKBP_EVENT_SYSRQ event?
> +
> if (ckdev->ec->event_size != ckdev->cols) {
> dev_err(ckdev->dev,
> "Discarded incomplete key matrix event.\n");
> @@ -270,13 +274,17 @@ static int cros_ec_keyb_work(struct notifier_block *nb,
> case EC_MKBP_EVENT_BUTTON:
> case EC_MKBP_EVENT_SWITCH:
> /*
> - * If EC is not the wake source, discard key state
> + * If bs is not wake enabled, discard key state
> * changes during suspend. Switches will be re-checked in
> * cros_ec_keyb_resume() to be sure nothing is lost.
> */
> - if (queued_during_suspend)
> + if (queued_during_suspend && ckdev->bs_idev
> + && !device_may_wakeup(&ckdev->bs_idev->dev))
> return NOTIFY_OK;
>
> + else if (queued_during_suspend && ckdev->bs_idev)
> + pm_wakeup_event(&ckdev->bs_idev->dev, 0);
> +
> if (ckdev->ec->event_data.event_type == EC_MKBP_EVENT_BUTTON) {
> val = get_unaligned_le32(
> &ckdev->ec->event_data.data.buttons);
> @@ -522,6 +530,7 @@ static int cros_ec_keyb_register_bs(struct cros_ec_keyb *ckdev)
> return ret;
> }
>
> + device_init_wakeup(&idev->dev, true);
> return 0;
> }
>
> @@ -598,6 +607,7 @@ static int cros_ec_keyb_register_matrix(struct cros_ec_keyb *ckdev)
> return err;
> }
>
> + device_init_wakeup(&idev->dev, true);
> return 0;
> }
>
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index d61024141e2b6..df9520365e54b 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -64,12 +64,14 @@ static irqreturn_t ec_irq_thread(int irq, void *data)
> * cros_ec_get_next_event() returned an error (default value for
> * wake_event is true)
> */
> - if (wake_event && device_may_wakeup(ec_dev->dev))
> + if (device_may_wakeup(ec_dev->dev) && wake_event
> + && ec_dev->wake_enabled)
> pm_wakeup_event(ec_dev->dev, 0);
>
> if (ret > 0)
> blocking_notifier_call_chain(&ec_dev->event_notifier,
> - 0, ec_dev);
> + wake_event && ec_dev->wake_enabled,
> + ec_dev);
> return IRQ_HANDLED;
> }
>
> @@ -229,7 +231,7 @@ int cros_ec_suspend(struct cros_ec_device *ec_dev)
> }
> EXPORT_SYMBOL(cros_ec_suspend);
>
> -static void cros_ec_drain_events(struct cros_ec_device *ec_dev)
> +static void cros_ec_report_events_during_suspend(struct cros_ec_device *ec_dev)
> {
> while (cros_ec_get_next_event(ec_dev, NULL) > 0)
> blocking_notifier_call_chain(&ec_dev->event_notifier,
> @@ -253,21 +255,16 @@ int cros_ec_resume(struct cros_ec_device *ec_dev)
> dev_dbg(ec_dev->dev, "Error %d sending resume event to ec",
> ret);
>
> - /*
> - * In some cases, we need to distinguish between events that occur
> - * during suspend if the EC is not a wake source. For example,
> - * keypresses during suspend should be discarded if it does not wake
> - * the system.
> - *
> - * If the EC is not a wake source, drain the event queue and mark them
> - * as "queued during suspend".
> - */
> if (ec_dev->wake_enabled) {
> disable_irq_wake(ec_dev->irq);
> ec_dev->wake_enabled = 0;
> - } else {
> - cros_ec_drain_events(ec_dev);
> }
> + /*
> + * Let the mfd devices know about events that occur during
> + * suspend. This way the clients know what to do with them.
> + */
> + cros_ec_report_events_during_suspend(ec_dev);
> +
>
> return 0;
> }
> --
> 2.17.0.441.gb46fe60e1d-goog
>
--
Dmitry
^ permalink raw reply
* Re: [GIT PULL] IB chrome-platform/input for atmel_mxt_ts & chromeos_laptop for device properties
From: Dmitry Torokhov @ 2018-05-24 23:18 UTC (permalink / raw)
To: Benson Leung; +Cc: bleung, linux-input, dtor, linux-kernel
In-Reply-To: <20180523195645.GA92470@decatoncale.mtv.corp.google.com>
On Wed, May 23, 2018 at 12:56:45PM -0700, Benson Leung wrote:
> Hi Dmitry,
>
> The following changes since commit 96a938aa214e965d5b4a2f10443b29cad14289b9:
>
> Input: atmel_mxt_ts - remove platform data support (2018-04-09 22:55:16 -0700)
>
> are available in the Git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/bleung/chrome-platform.git ib-chrome-platform-atmel-mxt-ts-device-properties
>
> for you to fetch changes up to 5020cd29d8bfcb3f3add43ea7d58b07011ab96d8:
>
> platform/chrome: chromeos_laptop - supply properties for ACPI devices (2018-05-23 12:48:31 -0700)
>
> ----------------------------------------------------------------
> Dmitry Torokhov (1):
> platform/chrome: chromeos_laptop - supply properties for ACPI devices
>
> drivers/platform/chrome/chromeos_laptop.c | 307 +++++++++++++++++++++++++++---
> 1 file changed, 278 insertions(+), 29 deletions(-)
Thanks Benson!
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2 1/3] input: touchscreen: edt-ft5x06: don't make device a wakeup source by default
From: Dmitry Torokhov @ 2018-05-24 23:17 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
Sascha Hauer, linux-input, Fabio Estevam, Shawn Guo, Daniel Mack
In-Reply-To: <CAL_JsqJQt6vFqH-KzNvjW+FZXCgzNAwXj_nSYow2Uz2Vcmi_Cg@mail.gmail.com>
On Wed, May 23, 2018 at 09:45:05AM -0500, Rob Herring wrote:
> On Wed, May 23, 2018 at 3:27 AM, Daniel Mack <daniel@zonque.org> wrote:
> > On Tuesday, May 22, 2018 07:54 PM, Rob Herring wrote:
> >>
> >> On Thu, May 17, 2018 at 11:05:50AM +0200, Daniel Mack wrote:
> >>>
> >>> Allow configuring the device as wakeup source through device properties,
> >>> as
> >>> not all platforms want to wake up on touch screen activity.
> >>>
> >>> The I2C core automatically reads the "wakeup-source" DT property to
> >>> configure a device's wakeup capability, and board supports files can set
> >>> I2C_CLIENT_WAKE in the flags.
> >>
> >>
> >> This will break wake-up on working systems. Looks like mostly i.MX, but
> >> there's one AM437x board. If that board doesn't care, then it is up to
> >> Shawn.
> >
> >
> > I added the property to the dts files, but as Dmitry pointed out, I missed
> > some. Sorry for that.
>
> Just adding the property to dts files doesn't fix the compatibility
> problem. If a user uses an existing dtb (before this change) with a
> new kernel (after this change), then wakeup will stop working.
Is this a practical problem though? Do we know of any products with
this touch panel that use DTS not distributed with the kernel?
--
Dmitry
^ permalink raw reply
* Re: [PATCH 3/6] ASoC: ams_delta: use GPIO lookup table
From: Janusz Krzysztofik @ 2018-05-24 20:35 UTC (permalink / raw)
To: Tony Lindgren
Cc: linux-fbdev, alsa-devel, Aaro Koskinen, Richard Weinberger,
Mark Brown, Dmitry Torokhov, Liam Girdwood, linux-kernel,
Peter Ujfalusi, Boris Brezillon, Tomi Valkeinen, linux-mtd,
linux-arm-kernel, linux-input, linux-omap, Jarkko Nikula
In-Reply-To: <20180523185244.GT98604@atomide.com>
On Wednesday, May 23, 2018 8:52:44 PM CEST Tony Lindgren wrote:
> * Mark Brown <broonie@kernel.org> [180521 10:07]:
> > On Fri, May 18, 2018 at 11:09:51PM +0200, Janusz Krzysztofik wrote:
> > > Now as the Amstrad Delta board provides GPIO lookup tables, switch from
> > > GPIO numbers to GPIO descriptors and use the table to locate required
> > > GPIO pins.
> >
> > Acked-by: Mark Brown <broonie@kernel.org>
>
Hi Tony,
> Thanks applying patches 1 and 3 of this series into omap-for-v4.18/soc.
> It's kind of getting late for v4.18, but let's see if we can still make
> it.
Thank you.
> Seems the others can be applied to the driver trees after no more
> comments, then once all that is done we can apply the last patch
> in this series.
I'll be submitting v2 of 5/6 (nand) very soon. 4/6 (lcd) is still waiting for
Tomi to respond. I hope there will be no issues with it. Howevver, regarding
2/6 - serio - I have to work more on that to satisfy Dmitry's comments. So
let's forget about 6/6 for now and I'll resubmit it again when we are ready
for that. Meanwhile, I'm going to submit a few more patches against the board
init file to complete migration to GPIO descriptors so dynamic allocation of
GPIO numbers to ams-delta latches will be possible.
Thanks,
Janusz
^ permalink raw reply
* Re: [PATCHv2] Input: atmel_mxt_ts - fix reset-gpio for level based irqs
From: Dmitry Torokhov @ 2018-05-24 16:35 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Nick Dyer, linux-input, linux-kernel, Ezequiel Garcia, kernel
In-Reply-To: <20180423115844.26281-1-sebastian.reichel@collabora.co.uk>
On Mon, Apr 23, 2018 at 01:58:44PM +0200, Sebastian Reichel wrote:
> The current reset-gpio support triggers an interrupt storm on platforms
> using the maxtouch with level based interrupt. The Motorola Droid 4,
> which I used for some of the tests is not affected, since it uses a edge
> based interrupt.
>
> This change avoids the interrupt storm by enabling the device while its
> interrupt is disabled. Afterwards we wait 100ms. This is important for
> two reasons: The device is unresponsive for some time (~22ms for
> mxt224E) and the CHG (interrupt) line is not working properly for 100ms.
> We don't need to wait for any following interrupts, since the following
> mxt_initialize() checks for bootloader mode anyways.
>
> This fixes a boot issue on GE PPD (watchdog kills device due to
> interrupt storm) and does not cause regression on Motorola Droid 4.
>
> Fixes: f657b00df22e ("Input: atmel_mxt_ts - add support for reset line")
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
Applied, thank you.
> ---
> Changes since PATCHv2:
> * fix patch description
> * use same timeout value for soft and gpio-reset
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 18 +++++++-----------
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 5d9699fe1b55..f97fbb0af020 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -194,6 +194,8 @@ enum t100_type {
>
> /* Delay times */
> #define MXT_BACKUP_TIME 50 /* msec */
> +#define MXT_RESET_GPIO_TIME 20 /* msec */
> +#define MXT_RESET_INVALID_CHG 100 /* msec */
> #define MXT_RESET_TIME 200 /* msec */
> #define MXT_RESET_TIMEOUT 3000 /* msec */
> #define MXT_CRC_TIMEOUT 1000 /* msec */
> @@ -1206,7 +1208,7 @@ static int mxt_soft_reset(struct mxt_data *data)
> return ret;
>
> /* Ignore CHG line for 100ms after reset */
> - msleep(100);
> + msleep(MXT_RESET_INVALID_CHG);
>
> mxt_acquire_irq(data);
>
> @@ -3167,20 +3169,14 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
> return error;
> }
>
> + disable_irq(client->irq);
> +
> if (data->reset_gpio) {
> - data->in_bootloader = true;
> - msleep(MXT_RESET_TIME);
> - reinit_completion(&data->bl_completion);
> + msleep(MXT_RESET_GPIO_TIME);
> gpiod_set_value(data->reset_gpio, 1);
> - error = mxt_wait_for_completion(data, &data->bl_completion,
> - MXT_RESET_TIMEOUT);
> - if (error)
> - return error;
> - data->in_bootloader = false;
> + msleep(MXT_RESET_INVALID_CHG);
> }
>
> - disable_irq(client->irq);
> -
> error = mxt_initialize(data);
> if (error)
> return error;
> --
> 2.17.0
>
--
Dmitry
^ permalink raw reply
* Re: Sometimes unusable i2c-hid devices in 4.17-rcX
From: Benjamin Tissoires @ 2018-05-24 13:57 UTC (permalink / raw)
To: Mario.Limonciello, Jason Gerecke; +Cc: linux-input, linux-kernel
In-Reply-To: <99e6ee5dcc47487f8d4463a1457fe74f@ausx13mpc120.AMER.DELL.COM>
Hi Mario,
On Tue, May 22, 2018 at 6:15 PM, <Mario.Limonciello@dell.com> wrote:
> Benjamin,
>
>> -----Original Message-----
>> From: Benjamin Tissoires [mailto:benjamin.tissoires@gmail.com]
>> Sent: Friday, May 18, 2018 1:18 PM
>> To: Limonciello, Mario
>> Cc: linux-input; linux-kernel@vger.kernel.org
>> Subject: Re: Sometimes unusable i2c-hid devices in 4.17-rcX
>>
>> On Thu, May 17, 2018 at 4:44 PM, <Mario.Limonciello@dell.com> wrote:
>> >> -----Original Message-----
>> >> From: Benjamin Tissoires [mailto:benjamin.tissoires@gmail.com]
>> >> Sent: Thursday, May 17, 2018 9:28 AM
>> >> To: Limonciello, Mario
>> >> Cc: linux-input; linux-kernel@vger.kernel.org
>> >> Subject: Re: Sometimes unusable i2c-hid devices in 4.17-rcX
>> >>
>> >> Hi Mario,
>> >>
>> >> On Wed, May 16, 2018 at 10:00 PM, <Mario.Limonciello@dell.com> wrote:
>> >> > Hi All,
>> >> >
>> >> > I've been running 4.16-rc7 on an XPS 9365 for some time and recently moved
>> up
>> >> to 4.17-rc5.
>> >> > Immediately I noticed that i2c-hid devices (both touchscreen and touchpad)
>> were
>> >> not working.
>> >> > Also when shutting the system down or rebooting it would just hang. (magic
>> sysrq
>> >> still worked).
>> >> >
>> >> > I figured it was an easy to identify regression so I started a bisect but it came
>> up
>> >> with garbage
>> >> > that ended in selftests shortly after 4.17-rc2. I realized that's because is still
>> will
>> >> fail on 4.17-rc2
>> >> > occasionally, seemingly after trying something newer and warm rebooting.
>> >> > So it seems like it's "worse" after 4.17-rc2 (doesn't work at all) but semi
>> >> reproducible on 4.17-rc2.
>> >> >
>> >> > Not sure if I'm chasing some initialization race, but wanted to see if anyone
>> else
>> >> was running into this
>> >> > or has some ideas?
>> >>
>> >> I am reliably running a v4.17-rc3 with a merge on Jiri's tree on the 9360.
>> >>
>> >> I doubt it's related to the event processing as I am not encountering
>> >> those issues.
>> >>
>> >> It *could* be related to the interrupts not being properly raised.
>> >>
>> >> Could you monitor /proc/interrupts and check if the ones associated
>> >> with your i2c-hid devices are increasing when you are using them?
>> >> Also, does the device emits raw HID events? (you can use hid-recorder
>> >> to check on the hidraw nodes.)
>> >
>>
>> Sorry, I couldn't get to it today. Monday is a public holiday here, so
>> I'll check on this Tuesday.
>>
>> > I checked both, /proc/interrupts isn't incrementing at all with the DLL077A:01
>> device.
>> > Hid-recorder is showing output from the raw HID node.
>>
>> I don't really understand how the hidraw node can send data while the
>> interrupts are not raised.
>>
>> Could you share the output of hid-recorder?
> Sure attached.
> Note that I had a dock connected at the same time since I needed power. This was
> different than my previous tests.
> That dock has 2 HID endpoints (so that might muddy this, I can re-capture if you need me to)
According to the file, this is the touchpad, and you draw a quick
circle on it. It's surprising the interrupts are not incrementing
because it clearly works fine.
>
>>
>> >
>> > Same thing for the touchscreen, no incrementing for it on the i2c_designware.0
>> device.
>> >
>> > Something notable however;
>> > When in this bad state hid-recorder didn't show /dev/hidraw1 for the
>> touchscreen (which
>> > Happens to be a Wacom touch screen).
>> > It only showed /dev/hidraw0 for the touchpad.
>>
>> This explains why the touchscreen doesn't increment the interrupts.
>> Something I missed in the first email is that the hidraw0 node
>> disappear for the wacom device as the touchpad gets the hidraw0 name.
>>
>> Could you provide the output of a working kernel configuration of:
>> sudo hid-recorder /dev/hidraw*
>>
>> This should provide me the whole logs at the same time of all the
>> hidraw nodes, and will allow me to reproduce the combination of
>> wacom/hid-multitouch you are experiencing.
>>
> I was having a hard time getting it to work again with 4.17-rcX while trying
> to capture this.
> The only thing I got it to work with was when I turned off the touchscreen
> In FW setup.
Yeah. Sorry I should have been more precise. Ideally, could you try to
capture all the hidraw nodes at once on a working kernel (4.16 or
4.15)?
I shouldn't need the current broken behavior, just a description (with
some events) of the devices as they are supposed to be working.
>
> So I guess that means it's probably something Wacom race condition on
> initialization since you noted the hidraw endpoint getting clobbered.
Still, it's curious the wacom module disappear without any message.
Jason, did you experienced such issues with some of the integrated
wacom panels on v4.17-rc3 and later?
Cheers,
Benjamin
>> >> >
>> >> > #dmesg | grep 'i2c\|hid' doesn't show any obvious errors when in this state of
>> >> non functional hid stuff.
>> >> > [ 2.398649] i2c /dev entries driver
>> >> > [ 2.881651] hidraw: raw HID events driver (C) Jiri Kosina
>> >> > [ 3.683583] ish-hid {33AECD58-B679-4E54-9BD9-A04D34F0C226}: [hid-ish]:
>> >> enum_devices_done OK, num_hid_devices=5
>> >> > [ 3.701259] hid-generic 001F:8086:22D8.0001: hidraw0: <UNKNOWN> HID
>> >> v2.00 Device [hid-ishtp 8086:22D8] on
>> >> > [ 3.702204] hid-generic 001F:8086:22D8.0002: hidraw1: <UNKNOWN> HID
>> >> v2.00 Device [hid-ishtp 8086:22D8] on
>> >> > [ 3.703063] hid-generic 001F:8086:22D8.0003: hidraw2: <UNKNOWN> HID
>> >> v2.00 Device [hid-ishtp 8086:22D8] on
>> >> > [ 3.704276] hid-generic 001F:8086:22D8.0004: hidraw3: <UNKNOWN> HID
>> >> v2.00 Device [hid-ishtp 8086:22D8] on
>> >> > [ 3.704557] hid-generic 001F:8086:22D8.0005: hidraw4: <UNKNOWN> HID
>> >> v2.00 Device [hid-ishtp 8086:22D8] on
>> >> > [ 3.750710] psmouse serio1: synaptics: Your touchpad (PNP: DLL077a
>> PNP0f13)
>> >> says it can support a different bus. If i2c-hid and hid-rmi are not used, you might
>> >> want to try setting psmouse.synaptics_intertouch to 1 and report this to linux-
>> >> input@vger.kernel.org.
>> >> > [ 7.030446] acpi INT33D5:00: intel-hid: created platform device
>> >> > [ 7.199178] i2c_hid i2c-WCOM482F:00: i2c-WCOM482F:00 supply vdd not
>> >> found, using dummy regulator
>> >> > [ 7.246638] input: WCOM482F:00 056A:482F as
>> >> /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-6/i2c-
>> >> WCOM482F:00/0018:056A:482F.0006/input/input11
>> >> > [ 7.246873] hid-generic 0018:056A:482F.0006: input,hidraw0: I2C HID v1.00
>> >> Mouse [WCOM482F:00 056A:482F] on i2c-WCOM482F:00
>> >> > [ 7.275279] i2c_hid i2c-DLL077A:01: i2c-DLL077A:01 supply vdd not found,
>> >> using dummy regulator
>> >> > [ 7.304107] input: DLL077A:01 06CB:76AF as
>> >> /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-7/i2c-
>> >> DLL077A:01/0018:06CB:76AF.0007/input/input14
>> >> > [ 7.304212] hid-generic 0018:06CB:76AF.0007: input,hidraw1: I2C HID v1.00
>> >> Mouse [DLL077A:01 06CB:76AF] on i2c-DLL077A:01
>> >> > [ 7.657123] usbcore: registered new interface driver usbhid
>> >> > [ 7.657124] usbhid: USB HID core driver
>> >> > [ 7.722876] input: Wacom HID 482F Pen as
>> >> /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-6/i2c-
>> >> WCOM482F:00/0018:056A:482F.0006/input/input15
>> >> > [ 7.723148] input: Wacom HID 482F Finger as
>> >> /devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-6/i2c-
>> >> WCOM482F:00/0018:056A:482F.0006/input/input16
>> >> > [ 7.723611] wacom 0018:056A:482F.0006: hidraw0: I2C HID v1.00 Mouse
>> >> [WCOM482F:00 056A:482F] on i2c-WCOM482F:00
>> >> > [ 7.768275] input: DLL077A:01 06CB:76AF Touchpad as
>> >> /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-7/i2c-
>> >> DLL077A:01/0018:06CB:76AF.0007/input/input19
>> >> > [ 7.864201] hid-multitouch 0018:06CB:76AF.0007: input,hidraw0: I2C HID
>> v1.00
>> >> Mouse [DLL077A:01 06CB:76AF] on i2c-DLL077A:01
>> >> >
>> >> > However in this state, I can't rmmod i2c-hid. It just hangs the system with this
>> >> trace:
>> >> > [ 243.033779] INFO: task kworker/u8:0:6 blocked for more than 120 seconds.
>> >> > [ 243.033793] Not tainted 4.17.0-rc1+ #37
>> >> > [ 243.033798] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
>> this
>> >> message.
>> >> > [ 243.033804] kworker/u8:0 D 0 6 2 0x80000000
>> >> > [ 243.033826] Workqueue: events_power_efficient
>> >> power_supply_deferred_register_work
>> >> > [ 243.033832] Call Trace:
>> >> > [ 243.033850] __schedule+0x3c2/0x890
>> >> > [ 243.033861] ? __switch_to_asm+0x40/0x70
>> >> > [ 243.033868] schedule+0x36/0x80
>> >> > [ 243.033875] schedule_preempt_disabled+0xe/0x10
>> >> > [ 243.033882] __mutex_lock.isra.4+0x2ae/0x4e0
>> >> > [ 243.033890] ? __switch_to_asm+0x34/0x70
>> >> > [ 243.033899] ? __switch_to_asm+0x40/0x70
>> >> > [ 243.033906] ? __switch_to_asm+0x40/0x70
>> >> > [ 243.033914] __mutex_lock_slowpath+0x13/0x20
>> >> > [ 243.033920] ? __mutex_lock_slowpath+0x13/0x20
>> >> > [ 243.033927] mutex_lock+0x2f/0x40
>> >> > [ 243.033933] power_supply_deferred_register_work+0x2b/0x50
>> >> > [ 243.033944] process_one_work+0x148/0x3d0
>> >> > [ 243.033952] worker_thread+0x4b/0x460
>> >> > [ 243.033960] kthread+0x102/0x140
>> >> > [ 243.033967] ? rescuer_thread+0x380/0x380
>> >> > [ 243.033973] ? kthread_associate_blkcg+0xa0/0xa0
>> >> > [ 243.033982] ret_from_fork+0x35/0x40
>> >> > [ 243.034012] INFO: task systemd-udevd:308 blocked for more than 120
>> seconds.
>> >> > [ 243.034018] Not tainted 4.17.0-rc1+ #37
>> >> > [ 243.034022] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
>> this
>> >> message.
>> >> > [ 243.034027] systemd-udevd D 0 308 279 0x80000104
>> >> > [ 243.034033] Call Trace:
>> >> > [ 243.034041] __schedule+0x3c2/0x890
>> >> > [ 243.034049] schedule+0x36/0x80
>> >> > [ 243.034056] schedule_timeout+0x1e7/0x360
>> >> > [ 243.034066] ? ttwu_do_activate+0x77/0x80
>> >> > [ 243.034074] wait_for_completion+0xb4/0x140
>> >> > [ 243.034082] ? wake_up_q+0x70/0x70
>> >> > [ 243.034090] flush_work+0x12a/0x1e0
>> >> > [ 243.034097] ? worker_detach_from_pool+0xb0/0xb0
>> >> > [ 243.034107] __cancel_work_timer+0x112/0x190
>> >> > [ 243.034116] cancel_delayed_work_sync+0x13/0x20
>> >> > [ 243.034122] power_supply_unregister+0x37/0xb0
>> >> > [ 243.034127] devm_power_supply_release+0x11/0x20
>> >> > [ 243.034135] release_nodes+0x107/0x1f0
>> >> > [ 243.034147] devres_release_group+0x7c/0xb0
>> >> > [ 243.034162] wacom_remove+0xce/0x120 [wacom]
>> >> > [ 243.034178] hid_device_remove+0x4d/0xa0 [hid]
>> >> > [ 243.034187] device_release_driver_internal+0x155/0x220
>> >> > [ 243.034198] ? __hid_bus_driver_added+0x40/0x40 [hid]
>> >> > [ 243.034208] ? hid_destroy_device+0x60/0x60 [hid]
>> >> > [ 243.034215] device_release_driver+0x12/0x20
>> >> > [ 243.034221] device_reprobe+0x30/0x50
>> >> > [ 243.034231] __hid_bus_reprobe_drivers+0x45/0x50 [hid]
>> >> > [ 243.034239] bus_for_each_dev+0x64/0xb0
>> >> > [ 243.034250] __hid_bus_driver_added+0x2c/0x40 [hid]
>> >> > [ 243.034256] bus_for_each_drv+0x67/0xb0
>> >> > [ 243.034267] __hid_register_driver+0x6f/0x80 [hid]
>> >> > [ 243.034275] ? 0xffffffffc07f5000
>> >> > [ 243.034287] mt_driver_init+0x23/0x1000 [hid_multitouch]
>> >> > [ 243.034296] do_one_initcall+0x4f/0x1ce
>> >> > [ 243.034303] ? _cond_resched+0x1a/0x50
>> >> > [ 243.034315] ? kmem_cache_alloc_trace+0xb8/0x1f0
>> >> > [ 243.034327] do_init_module+0x5f/0x219
>> >> > [ 243.034335] load_module+0x24c7/0x2be0
>> >> > [ 243.034348] __do_sys_finit_module+0xe5/0x120
>> >> > [ 243.034354] ? __do_sys_finit_module+0xe5/0x120
>> >> > [ 243.034363] __x64_sys_finit_module+0x1a/0x20
>> >> > [ 243.034370] do_syscall_64+0x54/0x110
>> >> > [ 243.034380] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> >> > [ 243.034386] RIP: 0033:0x7f2ec539c839
>> >> > [ 243.034390] RSP: 002b:00007ffcfc620298 EFLAGS: 00000246 ORIG_RAX:
>> >> 0000000000000139
>> >> > [ 243.034396] RAX: ffffffffffffffda RBX: 000055d6a0147ec0 RCX:
>> >> 00007f2ec539c839
>> >> > [ 243.034399] RDX: 0000000000000000 RSI: 00007f2ec507b0e5 RDI:
>> >> 000000000000000f
>> >> > [ 243.034402] RBP: 00007f2ec507b0e5 R08: 0000000000000000 R09:
>> >> 00007ffcfc6203b0
>> >> > [ 243.034405] R10: 000000000000000f R11: 0000000000000246 R12:
>> >> 0000000000000000
>> >> > [ 243.034408] R13: 000055d6a017b3a0 R14: 0000000000020000 R15:
>> >> 000055d6a0147ec0
>> >> > --
>> >> > 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 2/2] Input: atmel_mxt_ts - require device properties present when probing
From: Benson Leung @ 2018-05-23 20:03 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Benson Leung, Nick Dyer, linux-kernel
In-Reply-To: <20180504004135.67554-2-dmitry.torokhov@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6134 bytes --]
On Thu, May 03, 2018 at 05:41:35PM -0700, Dmitry Torokhov wrote:
> The driver needs help determining whether it is dealing with a touchscreen
> or a touchpad, and with button mapping. Previously we supported passing
> this data via device properties, and also had DMI lists for Chromebooks
> that specified Atmel devices in ACPI, but did not provide enough data
> there. Now that chromeos_laptop driver is adjusted to supply necessary
> device properties even for ACPI devices, we can drop the DMI tables and
> refuse to probe if device properties are not attached to the device.
>
> We use presence of "compatible" property to determine if device properties
> are attached to the device or not and rely on chromeos_laptop to re-probe
> the device after attaching missing device properties to it.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Looks good with the chromeos_laptop patch that preceeds this.
Reviewed-by: Benson Leung <bleung@chromium.org>
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 152 ++---------------------
> 1 file changed, 12 insertions(+), 140 deletions(-)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 09194721aed2d..4ac49c6c00060 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -2999,142 +2999,6 @@ static int mxt_parse_device_properties(struct mxt_data *data)
> return 0;
> }
>
> -#ifdef CONFIG_ACPI
> -
> -struct mxt_acpi_platform_data {
> - const char *hid;
> - const struct property_entry *props;
> -};
> -
> -static unsigned int samus_touchpad_buttons[] = {
> - KEY_RESERVED,
> - KEY_RESERVED,
> - KEY_RESERVED,
> - BTN_LEFT
> -};
> -
> -static const struct property_entry samus_touchpad_props[] = {
> - PROPERTY_ENTRY_U32_ARRAY("linux,gpio-keymap", samus_touchpad_buttons),
> - { }
> -};
> -
> -static struct mxt_acpi_platform_data samus_platform_data[] = {
> - {
> - /* Touchpad */
> - .hid = "ATML0000",
> - .props = samus_touchpad_props,
> - },
> - {
> - /* Touchscreen */
> - .hid = "ATML0001",
> - },
> - { }
> -};
> -
> -static unsigned int chromebook_tp_buttons[] = {
> - KEY_RESERVED,
> - KEY_RESERVED,
> - KEY_RESERVED,
> - KEY_RESERVED,
> - KEY_RESERVED,
> - BTN_LEFT
> -};
> -
> -static const struct property_entry chromebook_tp_props[] = {
> - PROPERTY_ENTRY_U32_ARRAY("linux,gpio-keymap", chromebook_tp_buttons),
> - { }
> -};
> -
> -static struct mxt_acpi_platform_data chromebook_platform_data[] = {
> - {
> - /* Touchpad */
> - .hid = "ATML0000",
> - .props = chromebook_tp_props,
> - },
> - {
> - /* Touchscreen */
> - .hid = "ATML0001",
> - },
> - { }
> -};
> -
> -static const struct dmi_system_id mxt_dmi_table[] = {
> - {
> - /* 2015 Google Pixel */
> - .ident = "Chromebook Pixel 2",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "Samus"),
> - },
> - .driver_data = samus_platform_data,
> - },
> - {
> - /* Samsung Chromebook Pro */
> - .ident = "Samsung Chromebook Pro",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "Google"),
> - DMI_MATCH(DMI_PRODUCT_NAME, "Caroline"),
> - },
> - .driver_data = samus_platform_data,
> - },
> - {
> - /* Other Google Chromebooks */
> - .ident = "Chromebook",
> - .matches = {
> - DMI_MATCH(DMI_SYS_VENDOR, "GOOGLE"),
> - },
> - .driver_data = chromebook_platform_data,
> - },
> - { }
> -};
> -
> -static int mxt_prepare_acpi_properties(struct i2c_client *client)
> -{
> - struct acpi_device *adev;
> - const struct dmi_system_id *system_id;
> - const struct mxt_acpi_platform_data *acpi_pdata;
> -
> - adev = ACPI_COMPANION(&client->dev);
> - if (!adev)
> - return -ENOENT;
> -
> - system_id = dmi_first_match(mxt_dmi_table);
> - if (!system_id)
> - return -ENOENT;
> -
> - acpi_pdata = system_id->driver_data;
> - if (!acpi_pdata)
> - return -ENOENT;
> -
> - while (acpi_pdata->hid) {
> - if (!strcmp(acpi_device_hid(adev), acpi_pdata->hid)) {
> - /*
> - * Remove previously installed properties if we
> - * are probing this device not for the very first
> - * time.
> - */
> - device_remove_properties(&client->dev);
> -
> - /*
> - * Now install the platform-specific properties
> - * that are missing from ACPI.
> - */
> - device_add_properties(&client->dev, acpi_pdata->props);
> - break;
> - }
> -
> - acpi_pdata++;
> - }
> -
> - return 0;
> -}
> -#else
> -static int mxt_prepare_acpi_properties(struct i2c_client *client)
> -{
> - return -ENOENT;
> -}
> -#endif
> -
> static const struct dmi_system_id chromebook_T9_suspend_dmi[] = {
> {
> .matches = {
> @@ -3155,6 +3019,18 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
> struct mxt_data *data;
> int error;
>
> + /*
> + * Ignore devices that do not have device properties attached to
> + * them, as we need help determining whether we are dealing with
> + * touch screen or touchpad.
> + *
> + * So far on x86 the only users of Atmel touch controllers are
> + * Chromebooks, and chromeos_laptop driver will ensure that
> + * necessary properties are provided (if firmware does not do that).
> + */
> + if (!device_property_present(&client->dev, "compatible"))
> + return -ENXIO;
> +
> /*
> * Ignore ACPI devices representing bootloader mode.
> *
> @@ -3186,10 +3062,6 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id)
> data->suspend_mode = dmi_check_system(chromebook_T9_suspend_dmi) ?
> MXT_SUSPEND_T9_CTRL : MXT_SUSPEND_DEEP_SLEEP;
>
> - error = mxt_prepare_acpi_properties(client);
> - if (error && error != -ENOENT)
> - return error;
> -
> error = mxt_parse_device_properties(data);
> if (error)
> return error;
> --
> 2.17.0.441.gb46fe60e1d-goog
>
--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [GIT PULL] IB chrome-platform/input for atmel_mxt_ts & chromeos_laptop for device properties
From: Benson Leung @ 2018-05-23 19:56 UTC (permalink / raw)
To: dmitry.torokhov, bleung, bleung, linux-input, dtor, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 976 bytes --]
Hi Dmitry,
The following changes since commit 96a938aa214e965d5b4a2f10443b29cad14289b9:
Input: atmel_mxt_ts - remove platform data support (2018-04-09 22:55:16 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/bleung/chrome-platform.git ib-chrome-platform-atmel-mxt-ts-device-properties
for you to fetch changes up to 5020cd29d8bfcb3f3add43ea7d58b07011ab96d8:
platform/chrome: chromeos_laptop - supply properties for ACPI devices (2018-05-23 12:48:31 -0700)
----------------------------------------------------------------
Dmitry Torokhov (1):
platform/chrome: chromeos_laptop - supply properties for ACPI devices
drivers/platform/chrome/chromeos_laptop.c | 307 +++++++++++++++++++++++++++---
1 file changed, 278 insertions(+), 29 deletions(-)
Thanks!
--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox