Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH 05/13] dt-bindings: leds: add DT bindings for max77650
From: Jacek Anaszewski @ 2019-01-20 16:28 UTC (permalink / raw)
  To: Bartosz Golaszewski, Rob Herring, Mark Rutland, Linus Walleij,
	Dmitry Torokhov, Pavel Machek, Lee Jones, Sebastian Reichel,
	Liam Girdwood, Mark Brown, Greg Kroah-Hartman
  Cc: linux-kernel, linux-gpio, devicetree, linux-input, linux-leds,
	linux-pm, Bartosz Golaszewski
In-Reply-To: <20190118134244.22253-6-brgl@bgdev.pl>

Hi Bartosz,

Thank you for the patch.

On 1/18/19 2:42 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> Add the DT binding document for the LEDs module of max77650.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>   .../bindings/leds/leds-max77650.txt           | 57 +++++++++++++++++++
>   1 file changed, 57 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/leds/leds-max77650.txt
> 
> diff --git a/Documentation/devicetree/bindings/leds/leds-max77650.txt b/Documentation/devicetree/bindings/leds/leds-max77650.txt
> new file mode 100644
> index 000000000000..822b8893bc20
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/leds/leds-max77650.txt
> @@ -0,0 +1,57 @@
> +LED driver for MAX77650 PMIC from Maxim Integrated.
> +
> +This module is part of the MAX77650 MFD device. For more details
> +see Documentation/devicetree/bindings/mfd/max77650.txt.
> +
> +The LED controller is represented as a sub-node of the PMIC node on
> +the device tree.
> +
> +This device has three current sinks.
> +
> +Required properties:
> +--------------------
> +- compatible:		Must be "maxim,max77650-leds"
> +- #address-cells:	Must be <1>.
> +- #size-cells:		Must be <0>.
> +
> +Each LED is represented as a sub-node of the LED-controller node. Up to
> +three sub-nodes can be defined.
> +
> +Required properties of the sub-node:
> +------------------------------------
> +
> +- reg:			Must be <0>, <1> or <2>.
> +
> +Optional properties of the sub-node:
> +------------------------------------
> +
> +- label:		See Documentation/devicetree/bindings/leds/common.txt
> +- linux,default-trigger: See Documentation/devicetree/bindings/leds/common.txt
> +
> +For more details, please refer to the generic GPIO DT binding document
> +<devicetree/bindings/gpio/gpio.txt>.
> +
> +Example:
> +--------
> +
> +	leds {
> +		compatible = "maxim,max77650-leds";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		led0 {

s/led0/led@0/

> +			reg = <0>;
> +			label = "max77650:blue:usr0";
> +		};
> +
> +		led1 {

s/led1/led@1/

> +			reg = <1>;
> +			label = "max77650:red:usr1";
> +			linux,default-trigger = "heartbeat";
> +		};
> +
> +		led2 {

s/led2/led@2/

> +			reg = <2>;
> +			label = "max77650:green:usr2";
> +		};

Please remove "max77650:" from labels and add it in the driver.

> +	};
> 

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply

* Re: [PATCH 11/13] leds: max77650: add LEDs support
From: Jacek Anaszewski @ 2019-01-20 16:39 UTC (permalink / raw)
  To: Bartosz Golaszewski, Rob Herring, Mark Rutland, Linus Walleij,
	Dmitry Torokhov, Pavel Machek, Lee Jones, Sebastian Reichel,
	Liam Girdwood, Mark Brown, Greg Kroah-Hartman
  Cc: linux-kernel, linux-gpio, devicetree, linux-input, linux-leds,
	linux-pm, Bartosz Golaszewski
In-Reply-To: <20190118134244.22253-12-brgl@bgdev.pl>

Hi Bartosz,

Thank you for the patch.

I have few minor issues below.

On 1/18/19 2:42 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> 
> This adds basic support for LEDs for the max77650 PMIC. The device has
> three current sinks for driving LEDs.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> ---
>   drivers/leds/Kconfig         |   6 ++
>   drivers/leds/Makefile        |   1 +
>   drivers/leds/leds-max77650.c | 162 +++++++++++++++++++++++++++++++++++
>   3 files changed, 169 insertions(+)
>   create mode 100644 drivers/leds/leds-max77650.c
> 
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index a72f97fca57b..6e7a8f51eccc 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -608,6 +608,12 @@ config LEDS_TLC591XX
>   	  This option enables support for Texas Instruments TLC59108
>   	  and TLC59116 LED controllers.
>   
> +config LEDS_MAX77650
> +	tristate "LED support for Maxim MAX77650 PMIC"
> +	depends on MFD_MAX77650
> +	help
> +	  LEDs driver for MAX77650 family of PMICs from Maxim Integrated."
> +
>   config LEDS_MAX77693
>   	tristate "LED support for MAX77693 Flash"
>   	depends on LEDS_CLASS_FLASH
> diff --git a/drivers/leds/Makefile b/drivers/leds/Makefile
> index 4c1b0054f379..f48b2404dbb7 100644
> --- a/drivers/leds/Makefile
> +++ b/drivers/leds/Makefile
> @@ -61,6 +61,7 @@ obj-$(CONFIG_LEDS_MC13783)		+= leds-mc13783.o
>   obj-$(CONFIG_LEDS_NS2)			+= leds-ns2.o
>   obj-$(CONFIG_LEDS_NETXBIG)		+= leds-netxbig.o
>   obj-$(CONFIG_LEDS_ASIC3)		+= leds-asic3.o
> +obj-$(CONFIG_LEDS_MAX77650)		+= leds-max77650.o
>   obj-$(CONFIG_LEDS_MAX77693)		+= leds-max77693.o
>   obj-$(CONFIG_LEDS_MAX8997)		+= leds-max8997.o
>   obj-$(CONFIG_LEDS_LM355x)		+= leds-lm355x.o
> diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
> new file mode 100644
> index 000000000000..be2bb1c60448
> --- /dev/null
> +++ b/drivers/leds/leds-max77650.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2018 BayLibre SAS
> + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> + *
> + * LEDS driver for MAXIM 77650/77651 charger/power-supply.
> + */

Please use uniform C++ comment style.

If in doubt, please refer to the following Linus statement
from [0]:

"And yes, feel free to replace block comments with // while at it."

> +#include <linux/i2c.h>
> +#include <linux/leds.h>
> +#include <linux/mfd/max77650.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define MAX77650_NUM_LEDS		3
> +
> +#define MAX77650_LED_A_BASE		0x40
> +#define MAX77650_LED_B_BASE		0x43
> +
> +#define MAX77650_LED_BR_MASK		GENMASK(4, 0)
> +#define MAX77650_LED_EN_MASK		GENMASK(7, 6)
> +
> +/* Enable EN_LED_MSTR. */
> +#define MAX77650_LED_TOP_DEFAULT	BIT(0)
> +
> +#define MAX77650_LED_ENABLE		GENMASK(7, 6)
> +#define MAX77650_LED_DISABLE		0x00
> +
> +#define MAX77650_LED_A_DEFAULT		MAX77650_LED_DISABLE
> +/* 100% on duty */
> +#define MAX77650_LED_B_DEFAULT		GENMASK(3, 0)
> +
> +struct max77650_leds;
> +
> +struct max77650_led {
> +	struct led_classdev cdev;
> +	struct max77650_leds *parent;
> +	unsigned int regA;
> +	unsigned int regB;
> +};
> +
> +struct max77650_leds {
> +	struct regmap *map;
> +	struct max77650_led leds[MAX77650_NUM_LEDS];
> +};
> +
> +static struct max77650_led *max77650_to_led(struct led_classdev *cdev)
> +{
> +	return container_of(cdev, struct max77650_led, cdev);
> +}
> +
> +static int max77650_leds_brightness_set(struct led_classdev *cdev,
> +					enum led_brightness brightness)

I would change function names to max77650_led* (singular).
It would be consistent with most of the other LED class drivers.
LED driver for max77693 flash cell also uses this scheme
(drivers/leds/leds-max77693.c).

> +{
> +	struct max77650_led *led = max77650_to_led(cdev);
> +	struct regmap *regmap = led->parent->map;
> +	int val, mask;
> +
> +	mask = MAX77650_LED_BR_MASK | MAX77650_LED_EN_MASK;
> +
> +	if (brightness == LED_OFF) {
> +		val = MAX77650_LED_DISABLE;
> +	} else {
> +		val = MAX77650_LED_ENABLE;
> +		/*
> +		 * We can set the brightness with 5-bit resolution.
> +		 *
> +		 * For brightness == 1, the bits we're writing will be 0, but
> +		 * since we keep LED_FS0 set to 12.8mA full-scale range, the
> +		 * LED will be lit slightly.
> +		 */
> +		val |= brightness / 8;
> +	}
> +
> +	return regmap_update_bits(regmap, led->regA, mask, val);
> +}
> +
> +static int max77650_leds_probe(struct platform_device *pdev)
> +{
> +	struct device_node *of_node, *child;
> +	struct max77650_leds *leds;
> +	struct max77650_led *led;
> +	struct device *parent;
> +	struct device *dev;
> +	int rv, num_leds;
> +	u32 reg;
> +
> +	dev = &pdev->dev;
> +	parent = dev->parent;
> +	of_node = dev->of_node;
> +
> +	if (!of_node)
> +		return -ENODEV;
> +
> +	leds = devm_kzalloc(dev, sizeof(*leds), GFP_KERNEL);
> +	if (!leds)
> +		return -ENOMEM;
> +
> +	leds->map = dev_get_regmap(dev->parent, NULL);
> +	if (!leds->map)
> +		return -ENODEV;
> +
> +	num_leds = of_get_child_count(of_node);
> +	if (!num_leds || num_leds > MAX77650_NUM_LEDS)
> +		return -ENODEV;
> +
> +	for_each_child_of_node(of_node, child) {
> +		rv = of_property_read_u32(child, "reg", &reg);
> +		if (rv || reg >= MAX77650_NUM_LEDS)
> +			return -EINVAL;
> +
> +		led = &leds->leds[reg];
> +
> +		led->parent = leds;
> +		led->regA = MAX77650_LED_A_BASE + reg;
> +		led->regB = MAX77650_LED_B_BASE + reg;
> +		led->cdev.brightness_set_blocking
> +					= max77650_leds_brightness_set;
> +
> +		led->cdev.name = of_get_property(child, "label", NULL);
> +		if (!led->cdev.name)
> +			led->cdev.name = child->name;

Please follow how other recent LED class drivers construct LED names,
e.g. drivers/leds/leds-cr0014114.c. We are in the course of creating
generic support for creating LED names, but until it is agreed upon
we've got to use other drivers as a reference.

> +
> +		of_property_read_string(child, "linux,default-trigger",
> +					&led->cdev.default_trigger);
> +
> +		rv = devm_of_led_classdev_register(dev, child, &led->cdev);
> +		if (rv)
> +			return rv;
> +
> +		rv = regmap_write(leds->map, led->regA,
> +				  MAX77650_LED_A_DEFAULT);
> +		if (rv)
> +			return rv;
> +
> +		rv = regmap_write(leds->map, led->regB,
> +				  MAX77650_LED_B_DEFAULT);
> +		if (rv)
> +			return rv;
> +	}
> +
> +	rv = regmap_write(leds->map,
> +			  MAX77650_REG_CNFG_LED_TOP,
> +			  MAX77650_LED_TOP_DEFAULT);
> +	if (rv)
> +		return rv;

No need to check rv here:

	return regmap_write(leds->map,
			  MAX77650_REG_CNFG_LED_TOP,
			  MAX77650_LED_TOP_DEFAULT)

> +	return 0;
> +}
> +
> +static struct platform_driver max77650_leds_driver = {
> +	.driver = {
> +		.name = "max77650-leds",

s/leds/led/

> +	},
> +	.probe = max77650_leds_probe,
> +};
> +module_platform_driver(max77650_leds_driver);
> +
> +MODULE_DESCRIPTION("MAXIM 77650/77651 LED driver");
> +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
> +MODULE_LICENSE("GPL");

s/GPL/GPL v2/


[0] https://lkml.org/lkml/2017/11/2/715

-- 
Best regards,
Jacek Anaszewski

^ permalink raw reply

* [PATCH] input: Fix the CONFIG_SPARC64 mixup
From: Deepa Dinamani @ 2019-01-21  1:49 UTC (permalink / raw)
  To: dmitry.torokhov; +Cc: y2038, davem, linux-kernel, arnd, linux-input

Arnd Bergmann pointed out that CONFIG_* cannot be used
in a uapi header. Override with an equivalent conditional.

Fixes: 2e746942ebac ("Input: input_event - provide override for sparc64")
Fixes: 152194fe9c3f ("Input: extend usable life of event timestamps to 2106 on 32 bit systems")
Signed-off-by: Deepa Dinamani <deepa.kernel@gmail.com>
---
 include/uapi/linux/input.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index ffab958bc512..f056b2a00d5c 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -32,7 +32,7 @@ struct input_event {
 #define input_event_usec time.tv_usec
 #else
 	__kernel_ulong_t __sec;
-#ifdef CONFIG_SPARC64
+#if defined(__sparc__) && defined(__arch64__)
 	unsigned int __usec;
 #else
 	__kernel_ulong_t __usec;
-- 
2.17.1

_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

^ permalink raw reply related

* Re: [PATCH] HID: i2c-hid: Disable runtime PM on Goodix touchpad
From: Kai-Heng Feng @ 2019-01-21  3:23 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Hans de Goede, anisse, Jiri Kosina, open list:HID CORE LAYER,
	lkml
In-Reply-To: <CAO-hwJ+OQAsJkJj2YRGOE=8NcXhS=XWwdfGSaLjY9Bdvcfn7BQ@mail.gmail.com>



> On Jan 17, 2019, at 20:35, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
> 
> On Thu, Jan 17, 2019 at 12:41 PM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
>> 
>> 
>> 
>>> On Jan 17, 2019, at 16:06, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
>>> 
>>> On Thu, Jan 17, 2019 at 6:02 AM Kai-Heng Feng
>>> <kai.heng.feng@canonical.com> wrote:
>> [snipped]
>>>> Goodix says the firmware needs at least 60ms to fully respond ON and
>>>> SLEEP command.
>>> 
>>> I was about to say that this is not conformant to the specification.
>>> But looking at 7.2.8 SET_POWER of
>>> https://docs.microsoft.com/en-us/previous-versions/windows/hardware/design/dn642101(v=vs.85)
>>> 
>>> They say:
>>> "The DEVICE must ensure that it transitions to the HOST specified
>>> Power State in under 1 second. "
>>> But in the RESPONSE paragraph: "The DEVICE shall not respond back
>>> after receiving the command. The DEVICE is mandated to enter that
>>> power state imminently."
>>> 
>>> My interpretation was always that the device has to be in a ready
>>> state for new commands "immediately".
>>> However, the first sentence may suggest that the driver would block
>>> any command to the device before the 1 second timestamp.
>>> 
>>>> 
>>>> I’ll see if an 200ms autosuspend can solve all Goodix, LG and Raydium
>>>> touchpanels.
>>> 
>>> We already have a I2C_HID_QUIRK_DELAY_AFTER_SLEEP quirk that delay
>>> operations after sleep by 20ms. Maybe we can keep the runtime PM but
>>> use the same timers to not confuse the hardware.
>> 
>> Yes, but wouldn’t use pm_*_autosuspend() helpers can both solve the issue
>> and make the code more cleaner?
> 
> You are probably correct :)
> 
> I am not too familiar with the details of the runtime PM API, but if
> you can make this a barrier to prevent the host to call too many
> suspends/resumes in a row, that would be nice.
> And we might be able to ditch I2C_HID_QUIRK_DELAY_AFTER_SLEEP and
> I2C_HID_QUIRK_NO_RUNTIME_PM.

Ok, using autosuspend helpers doesn’t really solve the issue.

Although it can cover the case like fast ON/SLEEP triggered by
quick transition of display manager -> desktop session, but once
it gets runtime suspended, we can never sure when it’ll get 
runtime resumed again. So a mleep() between each powering
commands is still needed.

So I think we should stick to I2C_HID_QUIRK_NO_RUNTIME_PM
for now.

Kai-Heng

> 
> Adding the involved people in the thread.
> 
> Cheers,
> Benjamin

^ permalink raw reply

* Re: [PATCH v3] HID: i2c-hid: Ignore input report if there's no data present on Elan touchpanels
From: Kai-Heng Feng @ 2019-01-21  3:29 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Jiri Kosina, open list:HID CORE LAYER, lkml
In-Reply-To: <CAO-hwJKo=EaPoXJLGUvjq3QRng1V1NkO7A5zYGojbPyxxyJYHA@mail.gmail.com>



> On Jan 18, 2019, at 23:50, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote:
> 
> Hi Kai-Heng,
> 
> On Mon, Jan 7, 2019 at 8:24 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
>> 
>> While using Elan touchpads, the message floods:
>> [  136.138487] i2c_hid i2c-DELL08D6:00: i2c_hid_get_input: incomplete report (14/65535)
>> 
>> Though the message flood is annoying, the device it self works without
>> any issue. I suspect that the device in question takes too much time to
>> pull the IRQ back to high after I2C host has done reading its data.
>> 
>> Since the host receives all useful data, let's ignore the input report
>> when there's no data.
>> 
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> 
> Thanks for the v3.
> 
> This patch has just been brought to my attention, and I have one
> question before applying it:
> are those messages (without your patch) occurring all the time, or
> just after resume?

All the time.

The touchpad works fine though. The entire report is 0xff, so it can be
safely ignored.

> 
> I wonder if the pm_suspend delay we talked about in the other thread
> would fix that issue in a cleaner way.

I’ve replied in another thread, unfortunately it can’t.

We can introduce msleep() between each commands though, but I don’t
think it’s good either.

Kai-Heng

> 
> Cheers,
> Benjamin
> 
>> ---
>> v3:
>> Fix compiler error/warnings.
>> 
>> v2:
>> Use dev_warn_once() to warn the user about the situation.
>> 
>> drivers/hid/i2c-hid/i2c-hid-core.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>> 
>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
>> index 8555ce7e737b..2f940c1de616 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>> @@ -50,6 +50,7 @@
>> #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET       BIT(1)
>> #define I2C_HID_QUIRK_NO_RUNTIME_PM            BIT(2)
>> #define I2C_HID_QUIRK_DELAY_AFTER_SLEEP                BIT(3)
>> +#define I2C_HID_QUIRK_BOGUS_IRQ                        BIT(4)
>> 
>> /* flags */
>> #define I2C_HID_STARTED                0
>> @@ -179,6 +180,8 @@ static const struct i2c_hid_quirks {
>>                I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
>>        { USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
>>                I2C_HID_QUIRK_NO_RUNTIME_PM },
>> +       { USB_VENDOR_ID_ELAN, HID_ANY_ID,
>> +                I2C_HID_QUIRK_BOGUS_IRQ },
>>        { 0, 0 }
>> };
>> 
>> @@ -503,6 +506,12 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
>>                return;
>>        }
>> 
>> +       if (ihid->quirks & I2C_HID_QUIRK_BOGUS_IRQ && ret_size == 0xffff) {
>> +               dev_warn_once(&ihid->client->dev, "%s: IRQ triggered but "
>> +                             "there's no data\n", __func__);
>> +               return;
>> +       }
>> +
>>        if ((ret_size > size) || (ret_size < 2)) {
>>                dev_err(&ihid->client->dev, "%s: incomplete report (%d/%d)\n",
>>                        __func__, size, ret_size);
>> --
>> 2.17.1
>> 

^ permalink raw reply

* Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
From: Kai-Heng Feng @ 2019-01-21  4:28 UTC (permalink / raw)
  To: 廖崇榮
  Cc: Benjamin Tissoires, Dmitry Torokhov, open list:HID CORE LAYER,
	lkml
In-Reply-To: <008c01d4af10$5b157f20$11407d60$@emc.com.tw>



> On Jan 18, 2019, at 17:29, 廖崇榮 <kt.liao@emc.com.tw> wrote:
> 
> 
> 
> -----Original Message-----
> From: Kai Heng Feng [mailto:kai.heng.feng@canonical.com] 
> Sent: Friday, January 18, 2019 12:15 AM
> To: Benjamin Tissoires
> Cc: Dmitry Torokhov; 廖崇榮; open list:HID CORE LAYER; lkml
> Subject: Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
> 
> 
> 
>> On Jan 17, 2019, at 10:42 PM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>> 
>> Hi Kai-Heng,
>> 
>> On Thu, Jan 17, 2019 at 10:30 AM Kai-Heng Feng 
>> <kai.heng.feng@canonical.com> wrote:
>>> 
>>> There are some new HP laptops with Elantech touchpad don't support 
>>> multitouch.
>>> 
>>> Both ETP_BUS_SMB_HST_NTFY_ONLY and ETP_BUS_PS2_SMB_HST_NTFY devices 
>>> can use SMBus to support 5 fingers touch, so we need to chech them too.
>>> 
>>> So use elantech_use_host_notify() to do a more thouroughly check.
>>> 
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>> ---
>>> drivers/input/mouse/elantech.c | 58 
>>> +++++++++++++++++-----------------
>>> 1 file changed, 29 insertions(+), 29 deletions(-)
>>> 
>>> diff --git a/drivers/input/mouse/elantech.c 
>>> b/drivers/input/mouse/elantech.c index 9fe075c137dc..5bcf1c147eb1 
>>> 100644
>>> --- a/drivers/input/mouse/elantech.c
>>> +++ b/drivers/input/mouse/elantech.c
>>> @@ -1799,6 +1799,34 @@ static int elantech_create_smbus(struct psmouse
> *psmouse,
>>>                                 leave_breadcrumbs); }
>>> 
>>> +static bool elantech_use_host_notify(struct psmouse *psmouse,
>>> +                                    struct elantech_device_info 
>>> +*info) {
>>> +       if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>>> +               return true;
>>> +
>>> +       switch (info->bus) {
>>> +       case ETP_BUS_PS2_ONLY:
>>> +               /* expected case */
>>> +               break;
>>> +       case ETP_BUS_SMB_ALERT_ONLY:
>>> +               /* fall-through  */
>>> +       case ETP_BUS_PS2_SMB_ALERT:
>>> +               psmouse_dbg(psmouse, "Ignoring SMBus provider through
> alert protocol.\n");
>>> +               break;
>>> +       case ETP_BUS_SMB_HST_NTFY_ONLY:
>>> +               /* fall-through  */
>>> +       case ETP_BUS_PS2_SMB_HST_NTFY:
>>> +               return true;
>>> +       default:
>>> +               psmouse_dbg(psmouse,
>>> +                           "Ignoring SMBus bus provider %d.\n",
>>> +                           info->bus);
>>> +       }
>>> +
>>> +       return false;
>>> +}
>>> +
>>> /**
>>> * elantech_setup_smbus - called once the PS/2 devices are enumerated
>>> * and decides to instantiate a SMBus InterTouch device.
>>> @@ -1818,7 +1846,7 @@ static int elantech_setup_smbus(struct psmouse
> *psmouse,
>>>                * i2c_blacklist_pnp_ids.
>>>                * Old ICs are up to the user to decide.
>>>                */
>>> -               if (!ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version) ||
>>> +               if (!elantech_use_host_notify(psmouse, info) ||
>> 
>> That was my initial approach of the series, but I ended up being more 
>> conservative as this would flip all of the existing elantech SMBUS 
>> capable touchpads to use elan_i2c.
>> And I didn't want to deal with 4/5 year old laptops that suddenly broke.
>> 
>> So I wonder if you can restrict this default change to the recent 
>> laptops (let's say 2018+). Maybe by looking at their FW version or 
>> something else in the DMI?
> 
> It was KT who told me that I should use ETP_BUS_PS2_SMB_HST_NTFY.
> 
> As for date, KT still knows better than me.
> 
> KT,
> Can you name a year which is safe enough to enable SMBus?
> 
> I have discussed it internally. 
> The internal rule for FW's SMbus implementation is stable after 2018
> If you meet some special case, please let me know.

Thanks for the info. I’ll use this for the V2 patch.

> 
> BTW, The SMbus supporting is requested by HP this time, and there are plenty
> of HP laptop use old IC
> which doesn't meet " ETP_NEW_IC_SMBUS_HOST_NOTIFY”.

One more question, does ETP_BUS_SMB_HST_NTFY_ONLY means
it should stick to SMBus, because it doesn’t support PS/2?

I’d like to merge all checks into elantech_use_host_notify() but
ETP_BUS_SMB_HST_NTFY_ONLY caught my attention.

Kai-Heng

> 
> Elan touchpad works well in PS/2 for HP, because it don't support
> TrackPoint.
> You may let old HP platform work as PS/2 for safety.
> 
> Thanks
> KT
> 
> Kai-Heng
> 
>> 
>> Cheers,
>> Benjamin
>> 
>>>                   psmouse_matches_pnp_id(psmouse,
> i2c_blacklist_pnp_ids))
>>>                       return -ENXIO;
>>>       }
>>> @@ -1838,34 +1866,6 @@ static int elantech_setup_smbus(struct psmouse
> *psmouse,
>>>       return 0;
>>> }
>>> 
>>> -static bool elantech_use_host_notify(struct psmouse *psmouse,
>>> -                                    struct elantech_device_info *info)
>>> -{
>>> -       if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>>> -               return true;
>>> -
>>> -       switch (info->bus) {
>>> -       case ETP_BUS_PS2_ONLY:
>>> -               /* expected case */
>>> -               break;
>>> -       case ETP_BUS_SMB_ALERT_ONLY:
>>> -               /* fall-through  */
>>> -       case ETP_BUS_PS2_SMB_ALERT:
>>> -               psmouse_dbg(psmouse, "Ignoring SMBus provider through
> alert protocol.\n");
>>> -               break;
>>> -       case ETP_BUS_SMB_HST_NTFY_ONLY:
>>> -               /* fall-through  */
>>> -       case ETP_BUS_PS2_SMB_HST_NTFY:
>>> -               return true;
>>> -       default:
>>> -               psmouse_dbg(psmouse,
>>> -                           "Ignoring SMBus bus provider %d.\n",
>>> -                           info->bus);
>>> -       }
>>> -
>>> -       return false;
>>> -}
>>> -
>>> int elantech_init_smbus(struct psmouse *psmouse) {
>>>       struct elantech_device_info info;
>>> --
>>> 2.17.1
> 

^ permalink raw reply

* RE: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
From: 廖崇榮 @ 2019-01-21  6:21 UTC (permalink / raw)
  To: 'Kai-Heng Feng'
  Cc: 'Benjamin Tissoires', 'Dmitry Torokhov',
	'open list:HID CORE LAYER', 'lkml'
In-Reply-To: <11585D81-F3E5-4924-A010-4B6A11919216@canonical.com>



-----Original Message-----
From: Kai-Heng Feng [mailto:kai.heng.feng@canonical.com] 
Sent: Monday, January 21, 2019 12:28 PM
To: 廖崇榮
Cc: Benjamin Tissoires; Dmitry Torokhov; open list:HID CORE LAYER; lkml
Subject: Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info



> On Jan 18, 2019, at 17:29, 廖崇榮 <kt.liao@emc.com.tw> wrote:
> 
> 
> 
> -----Original Message-----
> From: Kai Heng Feng [mailto:kai.heng.feng@canonical.com]
> Sent: Friday, January 18, 2019 12:15 AM
> To: Benjamin Tissoires
> Cc: Dmitry Torokhov; 廖崇榮; open list:HID CORE LAYER; lkml
> Subject: Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
> 
> 
> 
>> On Jan 17, 2019, at 10:42 PM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
>> 
>> Hi Kai-Heng,
>> 
>> On Thu, Jan 17, 2019 at 10:30 AM Kai-Heng Feng 
>> <kai.heng.feng@canonical.com> wrote:
>>> 
>>> There are some new HP laptops with Elantech touchpad don't support 
>>> multitouch.
>>> 
>>> Both ETP_BUS_SMB_HST_NTFY_ONLY and ETP_BUS_PS2_SMB_HST_NTFY devices 
>>> can use SMBus to support 5 fingers touch, so we need to chech them too.
>>> 
>>> So use elantech_use_host_notify() to do a more thouroughly check.
>>> 
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>>> ---
>>> drivers/input/mouse/elantech.c | 58
>>> +++++++++++++++++-----------------
>>> 1 file changed, 29 insertions(+), 29 deletions(-)
>>> 
>>> diff --git a/drivers/input/mouse/elantech.c 
>>> b/drivers/input/mouse/elantech.c index 9fe075c137dc..5bcf1c147eb1
>>> 100644
>>> --- a/drivers/input/mouse/elantech.c
>>> +++ b/drivers/input/mouse/elantech.c
>>> @@ -1799,6 +1799,34 @@ static int elantech_create_smbus(struct 
>>> psmouse
> *psmouse,
>>>                                 leave_breadcrumbs); }
>>> 
>>> +static bool elantech_use_host_notify(struct psmouse *psmouse,
>>> +                                    struct elantech_device_info
>>> +*info) {
>>> +       if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>>> +               return true;
>>> +
>>> +       switch (info->bus) {
>>> +       case ETP_BUS_PS2_ONLY:
>>> +               /* expected case */
>>> +               break;
>>> +       case ETP_BUS_SMB_ALERT_ONLY:
>>> +               /* fall-through  */
>>> +       case ETP_BUS_PS2_SMB_ALERT:
>>> +               psmouse_dbg(psmouse, "Ignoring SMBus provider 
>>> + through
> alert protocol.\n");
>>> +               break;
>>> +       case ETP_BUS_SMB_HST_NTFY_ONLY:
>>> +               /* fall-through  */
>>> +       case ETP_BUS_PS2_SMB_HST_NTFY:
>>> +               return true;
>>> +       default:
>>> +               psmouse_dbg(psmouse,
>>> +                           "Ignoring SMBus bus provider %d.\n",
>>> +                           info->bus);
>>> +       }
>>> +
>>> +       return false;
>>> +}
>>> +
>>> /**
>>> * elantech_setup_smbus - called once the PS/2 devices are enumerated
>>> * and decides to instantiate a SMBus InterTouch device.
>>> @@ -1818,7 +1846,7 @@ static int elantech_setup_smbus(struct psmouse
> *psmouse,
>>>                * i2c_blacklist_pnp_ids.
>>>                * Old ICs are up to the user to decide.
>>>                */
>>> -               if (!ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version) ||
>>> +               if (!elantech_use_host_notify(psmouse, info) ||
>> 
>> That was my initial approach of the series, but I ended up being more 
>> conservative as this would flip all of the existing elantech SMBUS 
>> capable touchpads to use elan_i2c.
>> And I didn't want to deal with 4/5 year old laptops that suddenly broke.
>> 
>> So I wonder if you can restrict this default change to the recent 
>> laptops (let's say 2018+). Maybe by looking at their FW version or 
>> something else in the DMI?
> 
> It was KT who told me that I should use ETP_BUS_PS2_SMB_HST_NTFY.
> 
> As for date, KT still knows better than me.
> 
> KT,
> Can you name a year which is safe enough to enable SMBus?
> 
> I have discussed it internally. 
> The internal rule for FW's SMbus implementation is stable after 2018 
> If you meet some special case, please let me know.

Thanks for the info. I’ll use this for the V2 patch.

> 
> BTW, The SMbus supporting is requested by HP this time, and there are 
> plenty of HP laptop use old IC which doesn't meet " 
> ETP_NEW_IC_SMBUS_HOST_NOTIFY”.

One more question, does ETP_BUS_SMB_HST_NTFY_ONLY means it should stick to SMBus, because it doesn’t support PS/2?

I’d like to merge all checks into elantech_use_host_notify() but ETP_BUS_SMB_HST_NTFY_ONLY caught my attention.

ETP_BUS_SMB_HST_NTFY_ONLY is for our previous planning but it never happen so far, and it won't happen in the future.
There are two cases for our released touchpad and they are " ETP_BUS_PS2_ONLY" and " ETP_BUS_PS2_SMB_HST_NTFY".

Thanks
KT

Kai-Heng

> 
> Elan touchpad works well in PS/2 for HP, because it don't support 
> TrackPoint.
> You may let old HP platform work as PS/2 for safety.
> 
> Thanks
> KT
> 
> Kai-Heng
> 
>> 
>> Cheers,
>> Benjamin
>> 
>>>                   psmouse_matches_pnp_id(psmouse,
> i2c_blacklist_pnp_ids))
>>>                       return -ENXIO;
>>>       }
>>> @@ -1838,34 +1866,6 @@ static int elantech_setup_smbus(struct 
>>> psmouse
> *psmouse,
>>>       return 0;
>>> }
>>> 
>>> -static bool elantech_use_host_notify(struct psmouse *psmouse,
>>> -                                    struct elantech_device_info *info)
>>> -{
>>> -       if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>>> -               return true;
>>> -
>>> -       switch (info->bus) {
>>> -       case ETP_BUS_PS2_ONLY:
>>> -               /* expected case */
>>> -               break;
>>> -       case ETP_BUS_SMB_ALERT_ONLY:
>>> -               /* fall-through  */
>>> -       case ETP_BUS_PS2_SMB_ALERT:
>>> -               psmouse_dbg(psmouse, "Ignoring SMBus provider through
> alert protocol.\n");
>>> -               break;
>>> -       case ETP_BUS_SMB_HST_NTFY_ONLY:
>>> -               /* fall-through  */
>>> -       case ETP_BUS_PS2_SMB_HST_NTFY:
>>> -               return true;
>>> -       default:
>>> -               psmouse_dbg(psmouse,
>>> -                           "Ignoring SMBus bus provider %d.\n",
>>> -                           info->bus);
>>> -       }
>>> -
>>> -       return false;
>>> -}
>>> -
>>> int elantech_init_smbus(struct psmouse *psmouse) {
>>>       struct elantech_device_info info;
>>> --
>>> 2.17.1
> 

^ permalink raw reply

* [PATCH 0/3] Stop managing the SP clock
From: Lubomir Rintel @ 2019-01-21  6:22 UTC (permalink / raw)
  To: Dmitry Torokhov, Michael Turquette
  Cc: Stephen Boyd, Rob Herring, Mark Rutland, linux-input, devicetree,
	linux-kernel, linux-clk

Hi,

Per discussion in [1] it seems that the kernel has no business managing
this clock: once the SP clock is disabled, it's not sufficient to just
enable it in order to bring the SP core back up.

Just let the firmware keep it enabled and don't expose it to drivers.

[1] https://lore.kernel.org/lkml/154783267051.169631.3197836544646625747@swboyd.mtv.corp.google.com/

The "secure processor" (SP) core runs the keyboard firmware on the
OLPC XO 1.75 laptop.

Lubo

^ permalink raw reply

* [PATCH 1/3] Revert "Input: olpc_apsp - enable the SP clock"
From: Lubomir Rintel @ 2019-01-21  6:22 UTC (permalink / raw)
  To: Dmitry Torokhov, Michael Turquette
  Cc: Stephen Boyd, Rob Herring, Mark Rutland, linux-input, devicetree,
	linux-kernel, linux-clk, Lubomir Rintel
In-Reply-To: <20190121062255.551587-1-lkundrak@v3.sk>

Turns out this is not such a great idea. Once the SP clock is disabled,
it's not sufficient to just enable in order to bring the SP core back up.

It seems that the kernel has no business managing this clock. Just let
the firmware keep it enabled.

This reverts commit ed22cee91a88c47e564478b012fdbcb079653499.

Link: https://lore.kernel.org/lkml/154783267051.169631.3197836544646625747@swboyd.mtv.corp.google.com/
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 .../devicetree/bindings/serio/olpc,ap-sp.txt       |  4 ----
 drivers/input/serio/olpc_apsp.c                    | 14 --------------
 2 files changed, 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/serio/olpc,ap-sp.txt b/Documentation/devicetree/bindings/serio/olpc,ap-sp.txt
index 36603419d6f8..0e72183f52bc 100644
--- a/Documentation/devicetree/bindings/serio/olpc,ap-sp.txt
+++ b/Documentation/devicetree/bindings/serio/olpc,ap-sp.txt
@@ -4,14 +4,10 @@ Required properties:
 - compatible : "olpc,ap-sp"
 - reg : base address and length of SoC's WTM registers
 - interrupts : SP-AP interrupt
-- clocks : phandle + clock-specifier for the clock that drives the WTM
-- clock-names:  should be "sp"
 
 Example:
 	ap-sp@d4290000 {
 		compatible = "olpc,ap-sp";
 		reg = <0xd4290000 0x1000>;
 		interrupts = <40>;
-		clocks = <&soc_clocks MMP2_CLK_SP>;
-		clock-names = "sp";
 	}
diff --git a/drivers/input/serio/olpc_apsp.c b/drivers/input/serio/olpc_apsp.c
index bae08226e3d9..a7cfab3db9ee 100644
--- a/drivers/input/serio/olpc_apsp.c
+++ b/drivers/input/serio/olpc_apsp.c
@@ -23,7 +23,6 @@
 #include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/delay.h>
-#include <linux/clk.h>
 
 /*
  * The OLPC XO-1.75 and XO-4 laptops do not have a hardware PS/2 controller.
@@ -75,7 +74,6 @@ struct olpc_apsp {
 	struct serio *kbio;
 	struct serio *padio;
 	void __iomem *base;
-	struct clk *clk;
 	int open_count;
 	int irq;
 };
@@ -148,17 +146,11 @@ static int olpc_apsp_open(struct serio *port)
 	struct olpc_apsp *priv = port->port_data;
 	unsigned int tmp;
 	unsigned long l;
-	int error;
 
 	if (priv->open_count++ == 0) {
-		error = clk_prepare_enable(priv->clk);
-		if (error)
-			return error;
-
 		l = readl(priv->base + COMMAND_FIFO_STATUS);
 		if (!(l & CMD_STS_MASK)) {
 			dev_err(priv->dev, "SP cannot accept commands.\n");
-			clk_disable_unprepare(priv->clk);
 			return -EIO;
 		}
 
@@ -179,8 +171,6 @@ static void olpc_apsp_close(struct serio *port)
 		/* Disable interrupt 0 */
 		tmp = readl(priv->base + PJ_INTERRUPT_MASK);
 		writel(tmp | INT_0, priv->base + PJ_INTERRUPT_MASK);
-
-		clk_disable_unprepare(priv->clk);
 	}
 }
 
@@ -208,10 +198,6 @@ static int olpc_apsp_probe(struct platform_device *pdev)
 	if (priv->irq < 0)
 		return priv->irq;
 
-	priv->clk = devm_clk_get(&pdev->dev, "sp");
-	if (IS_ERR(priv->clk))
-		return PTR_ERR(priv->clk);
-
 	/* KEYBOARD */
 	kb_serio = kzalloc(sizeof(struct serio), GFP_KERNEL);
 	if (!kb_serio)
-- 
2.20.1

^ permalink raw reply related

* [PATCH 2/3] Revert "clk: mmp2: add SP clock"
From: Lubomir Rintel @ 2019-01-21  6:22 UTC (permalink / raw)
  To: Dmitry Torokhov, Michael Turquette
  Cc: Stephen Boyd, Rob Herring, Mark Rutland, linux-input, devicetree,
	linux-kernel, linux-clk, Lubomir Rintel
In-Reply-To: <20190121062255.551587-1-lkundrak@v3.sk>

It seems that the kernel has no business managing this clock: once the SP
clock is disabled, it's not sufficient to just enable in order to bring the
SP core back up. Just let the firmware keep it enabled and don't expose it
to drivers.

This reverts commit fc27c2394d96fd19854b7e2d3f0e60df0d86fc90.

Link: https://lore.kernel.org/lkml/154783267051.169631.3197836544646625747@swboyd.mtv.corp.google.com/
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 drivers/clk/mmp/clk-of-mmp2.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/clk/mmp/clk-of-mmp2.c b/drivers/clk/mmp/clk-of-mmp2.c
index 61fefc046ec5..d083b860f083 100644
--- a/drivers/clk/mmp/clk-of-mmp2.c
+++ b/drivers/clk/mmp/clk-of-mmp2.c
@@ -53,7 +53,6 @@
 #define APMU_DISP1	0x110
 #define APMU_CCIC0	0x50
 #define APMU_CCIC1	0xf4
-#define APMU_SP		0x68
 #define MPMU_UART_PLL	0x14
 
 struct mmp2_clk_unit {
@@ -210,8 +209,6 @@ static struct mmp_clk_mix_config ccic1_mix_config = {
 	.reg_info = DEFINE_MIX_REG_INFO(4, 16, 2, 6, 32),
 };
 
-static DEFINE_SPINLOCK(sp_lock);
-
 static struct mmp_param_mux_clk apmu_mux_clks[] = {
 	{MMP2_CLK_DISP0_MUX, "disp0_mux", disp_parent_names, ARRAY_SIZE(disp_parent_names), CLK_SET_RATE_PARENT, APMU_DISP0, 6, 2, 0, &disp0_lock},
 	{MMP2_CLK_DISP1_MUX, "disp1_mux", disp_parent_names, ARRAY_SIZE(disp_parent_names), CLK_SET_RATE_PARENT, APMU_DISP1, 6, 2, 0, &disp1_lock},
@@ -242,7 +239,6 @@ static struct mmp_param_gate_clk apmu_gate_clks[] = {
 	{MMP2_CLK_CCIC1, "ccic1_clk", "ccic1_mix_clk", CLK_SET_RATE_PARENT, APMU_CCIC1, 0x1b, 0x1b, 0x0, 0, &ccic1_lock},
 	{MMP2_CLK_CCIC1_PHY, "ccic1_phy_clk", "ccic1_mix_clk", CLK_SET_RATE_PARENT, APMU_CCIC1, 0x24, 0x24, 0x0, 0, &ccic1_lock},
 	{MMP2_CLK_CCIC1_SPHY, "ccic1_sphy_clk", "ccic1_sphy_div", CLK_SET_RATE_PARENT, APMU_CCIC1, 0x300, 0x300, 0x0, 0, &ccic1_lock},
-	{MMP2_CLK_SP, "sp_clk", NULL, CLK_SET_RATE_PARENT, APMU_SP, 0x1b, 0x1b, 0x0, 0, &sp_lock},
 };
 
 static void mmp2_axi_periph_clk_init(struct mmp2_clk_unit *pxa_unit)
-- 
2.20.1

^ permalink raw reply related

* [PATCH 3/3] Revert "dt-bindings: marvell,mmp2: Add clock id for the SP clock"
From: Lubomir Rintel @ 2019-01-21  6:22 UTC (permalink / raw)
  To: Dmitry Torokhov, Michael Turquette
  Cc: Stephen Boyd, Rob Herring, Mark Rutland, linux-input, devicetree,
	linux-kernel, linux-clk, Lubomir Rintel
In-Reply-To: <20190121062255.551587-1-lkundrak@v3.sk>

It seems that the kernel has no business managing this clock: once the SP
clock is disabled, it's not sufficient to just enable it in order to bring
the SP core back up.

Pretty sure nothing ever used this and it's safe to remove.

This reverts commit e8a2c779141415105825e65a4715f1130bba61b1.

Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
 include/dt-bindings/clock/marvell,mmp2.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/dt-bindings/clock/marvell,mmp2.h b/include/dt-bindings/clock/marvell,mmp2.h
index 7b24fc791146..228a5e234af0 100644
--- a/include/dt-bindings/clock/marvell,mmp2.h
+++ b/include/dt-bindings/clock/marvell,mmp2.h
@@ -71,7 +71,6 @@
 #define MMP2_CLK_CCIC1_MIX		117
 #define MMP2_CLK_CCIC1_PHY		118
 #define MMP2_CLK_CCIC1_SPHY		119
-#define MMP2_CLK_SP			120
 
 #define MMP2_NR_CLKS			200
 #endif
-- 
2.20.1

^ permalink raw reply related

* [PATCH v2] Input: elantech: Enable SMBus on new (2018+) systems
From: Kai-Heng Feng @ 2019-01-21  7:02 UTC (permalink / raw)
  To: dmitry.torokhov
  Cc: benjamin.tissoires, kt.liao, linux-input, linux-kernel,
	Kai-Heng Feng

There are some new HP laptops with Elantech touchpad don't support
multitouch.

Currently we use ETP_NEW_IC_SMBUS_HOST_NOTIFY() to check if SMBus is
supported, but in addition to firmware version, the bus type also
informs us if the IC can support SMBus, so also check that.

In case of breaking old ICs, only enables SMBus on systems manufactured
after 2018, alongsides aforementioned checks.

Lastly, consolidats all check into elantech_use_host_notify() and use it
to determine whether to use PS/2 or SMBus.

Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
- Wording.
- Further restrain on older systems (< 2018).

 drivers/input/mouse/elantech.c | 63 ++++++++++++++++++----------------
 1 file changed, 34 insertions(+), 29 deletions(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 9fe075c137dc..2594130b0079 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1799,6 +1799,39 @@ static int elantech_create_smbus(struct psmouse *psmouse,
 				  leave_breadcrumbs);
 }
 
+static bool elantech_use_host_notify(struct psmouse *psmouse,
+				     struct elantech_device_info *info)
+{
+	bool host_notify = false;
+
+	if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
+		host_notify = true;
+	else {
+		switch (info->bus) {
+		case ETP_BUS_PS2_ONLY:
+			/* expected case */
+			break;
+		case ETP_BUS_SMB_ALERT_ONLY:
+			/* fall-through  */
+		case ETP_BUS_PS2_SMB_ALERT:
+			psmouse_dbg(psmouse, "Ignoring SMBus provider through alert protocol.\n");
+			break;
+		case ETP_BUS_SMB_HST_NTFY_ONLY:
+			/* fall-through  */
+		case ETP_BUS_PS2_SMB_HST_NTFY:
+			host_notify = true;
+			break;
+		default:
+			psmouse_dbg(psmouse,
+				    "Ignoring SMBus bus provider %d.\n",
+				    info->bus);
+		}
+	}
+
+	/* SMbus implementation is stable after 2018 */
+	return host_notify && (dmi_get_bios_year() >= 2018);
+}
+
 /**
  * elantech_setup_smbus - called once the PS/2 devices are enumerated
  * and decides to instantiate a SMBus InterTouch device.
@@ -1818,7 +1851,7 @@ static int elantech_setup_smbus(struct psmouse *psmouse,
 		 * i2c_blacklist_pnp_ids.
 		 * Old ICs are up to the user to decide.
 		 */
-		if (!ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version) ||
+		if (!elantech_use_host_notify(psmouse, info) ||
 		    psmouse_matches_pnp_id(psmouse, i2c_blacklist_pnp_ids))
 			return -ENXIO;
 	}
@@ -1838,34 +1871,6 @@ static int elantech_setup_smbus(struct psmouse *psmouse,
 	return 0;
 }
 
-static bool elantech_use_host_notify(struct psmouse *psmouse,
-				     struct elantech_device_info *info)
-{
-	if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
-		return true;
-
-	switch (info->bus) {
-	case ETP_BUS_PS2_ONLY:
-		/* expected case */
-		break;
-	case ETP_BUS_SMB_ALERT_ONLY:
-		/* fall-through  */
-	case ETP_BUS_PS2_SMB_ALERT:
-		psmouse_dbg(psmouse, "Ignoring SMBus provider through alert protocol.\n");
-		break;
-	case ETP_BUS_SMB_HST_NTFY_ONLY:
-		/* fall-through  */
-	case ETP_BUS_PS2_SMB_HST_NTFY:
-		return true;
-	default:
-		psmouse_dbg(psmouse,
-			    "Ignoring SMBus bus provider %d.\n",
-			    info->bus);
-	}
-
-	return false;
-}
-
 int elantech_init_smbus(struct psmouse *psmouse)
 {
 	struct elantech_device_info info;
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH v2] Input: elantech: Enable SMBus on new (2018+) systems
From: Benjamin Tissoires @ 2019-01-21  8:33 UTC (permalink / raw)
  To: Kai-Heng Feng
  Cc: Dmitry Torokhov, 廖崇榮,
	open list:HID CORE LAYER, lkml
In-Reply-To: <20190121070258.1844-1-kai.heng.feng@canonical.com>

On Mon, Jan 21, 2019 at 8:03 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> There are some new HP laptops with Elantech touchpad don't support
> multitouch.
>
> Currently we use ETP_NEW_IC_SMBUS_HOST_NOTIFY() to check if SMBus is
> supported, but in addition to firmware version, the bus type also
> informs us if the IC can support SMBus, so also check that.
>
> In case of breaking old ICs, only enables SMBus on systems manufactured
> after 2018, alongsides aforementioned checks.
>
> Lastly, consolidats all check into elantech_use_host_notify() and use it
> to determine whether to use PS/2 or SMBus.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v2:
> - Wording.
> - Further restrain on older systems (< 2018).
>
>  drivers/input/mouse/elantech.c | 63 ++++++++++++++++++----------------
>  1 file changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
> index 9fe075c137dc..2594130b0079 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1799,6 +1799,39 @@ static int elantech_create_smbus(struct psmouse *psmouse,
>                                   leave_breadcrumbs);
>  }
>
> +static bool elantech_use_host_notify(struct psmouse *psmouse,
> +                                    struct elantech_device_info *info)
> +{
> +       bool host_notify = false;
> +
> +       if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
> +               host_notify = true;
> +       else {
> +               switch (info->bus) {
> +               case ETP_BUS_PS2_ONLY:
> +                       /* expected case */
> +                       break;
> +               case ETP_BUS_SMB_ALERT_ONLY:
> +                       /* fall-through  */
> +               case ETP_BUS_PS2_SMB_ALERT:
> +                       psmouse_dbg(psmouse, "Ignoring SMBus provider through alert protocol.\n");
> +                       break;
> +               case ETP_BUS_SMB_HST_NTFY_ONLY:
> +                       /* fall-through  */
> +               case ETP_BUS_PS2_SMB_HST_NTFY:
> +                       host_notify = true;
> +                       break;
> +               default:
> +                       psmouse_dbg(psmouse,
> +                                   "Ignoring SMBus bus provider %d.\n",
> +                                   info->bus);
> +               }
> +       }
> +
> +       /* SMbus implementation is stable after 2018 */
> +       return host_notify && (dmi_get_bios_year() >= 2018);

Strictly speaking, the check for the year should be in the `switch
(info->bus)`, but OTOH, laptops with ETP_NEW_IC_SMBUS_HOST_NOTIFY
should be manufactured after 2018 too, so we should be good.

Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

Cheers,
Benjamin

> +}
> +
>  /**
>   * elantech_setup_smbus - called once the PS/2 devices are enumerated
>   * and decides to instantiate a SMBus InterTouch device.
> @@ -1818,7 +1851,7 @@ static int elantech_setup_smbus(struct psmouse *psmouse,
>                  * i2c_blacklist_pnp_ids.
>                  * Old ICs are up to the user to decide.
>                  */
> -               if (!ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version) ||
> +               if (!elantech_use_host_notify(psmouse, info) ||
>                     psmouse_matches_pnp_id(psmouse, i2c_blacklist_pnp_ids))
>                         return -ENXIO;
>         }
> @@ -1838,34 +1871,6 @@ static int elantech_setup_smbus(struct psmouse *psmouse,
>         return 0;
>  }
>
> -static bool elantech_use_host_notify(struct psmouse *psmouse,
> -                                    struct elantech_device_info *info)
> -{
> -       if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
> -               return true;
> -
> -       switch (info->bus) {
> -       case ETP_BUS_PS2_ONLY:
> -               /* expected case */
> -               break;
> -       case ETP_BUS_SMB_ALERT_ONLY:
> -               /* fall-through  */
> -       case ETP_BUS_PS2_SMB_ALERT:
> -               psmouse_dbg(psmouse, "Ignoring SMBus provider through alert protocol.\n");
> -               break;
> -       case ETP_BUS_SMB_HST_NTFY_ONLY:
> -               /* fall-through  */
> -       case ETP_BUS_PS2_SMB_HST_NTFY:
> -               return true;
> -       default:
> -               psmouse_dbg(psmouse,
> -                           "Ignoring SMBus bus provider %d.\n",
> -                           info->bus);
> -       }
> -
> -       return false;
> -}
> -
>  int elantech_init_smbus(struct psmouse *psmouse)
>  {
>         struct elantech_device_info info;
> --
> 2.17.1
>

^ permalink raw reply

* Re: [PATCH v1] HID: intel-ish-hid: Switch to use new generic UUID API
From: Christoph Hellwig @ 2019-01-21  8:40 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Srinivas Pandruvada, linux-input, Even Xu, linux-kernel,
	Christoph Hellwig
In-Reply-To: <20190110154804.89298-1-andriy.shevchenko@linux.intel.com>

On Thu, Jan 10, 2019 at 05:48:04PM +0200, Andy Shevchenko wrote:
> There are new types and helpers that are supposed to be used in new code.
> 
> As a preparation to get rid of legacy types and API functions do
> the conversion here.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/hid/intel-ish-hid/ishtp-hid-client.c |  3 +--
>  drivers/hid/intel-ish-hid/ishtp-hid.h        |  6 +++---
>  drivers/hid/intel-ish-hid/ishtp/bus.c        | 19 ++++++++-----------
>  drivers/hid/intel-ish-hid/ishtp/bus.h        |  4 ++--
>  drivers/hid/intel-ish-hid/ishtp/client.h     |  2 +-
>  drivers/hid/intel-ish-hid/ishtp/hbm.h        |  2 +-
>  6 files changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> index e64243bc9c96..2246697ada1d 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> @@ -788,8 +788,7 @@ static int hid_ishtp_cl_probe(struct ishtp_cl_device *cl_device)
>  	if (!cl_device)
>  		return	-ENODEV;
>  
> -	if (uuid_le_cmp(hid_ishtp_guid,
> -			cl_device->fw_client->props.protocol_name) != 0)
> +	if (!guid_equal(&hid_ishtp_guid, &cl_device->fw_client->props.protocol_name))

This adds an overly long line.

With that fixed:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply

* Re: [PATCH 1/7] Input: document meanings of KEY_SCREEN and KEY_ZOOM
From: Jiri Kosina @ 2019-01-21 10:11 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Benjamin Tissoires, Mauro Carvalho Chehab, linux-input,
	linux-kernel, linux-media
In-Reply-To: <20190118233037.87318-1-dmitry.torokhov@gmail.com>

On Fri, 18 Jan 2019, Dmitry Torokhov wrote:

> It is hard to say what KEY_SCREEN and KEY_ZOOM mean, but historically DVB
> folks have used them to indicate switch to full screen mode. Later, they
> converged on using KEY_ZOOM to switch into full screen mode and KEY)SCREEN
> to control aspect ratio (see Documentation/media/uapi/rc/rc-tables.rst).
> 
> Let's commit to these uses, and define:
> 
> - KEY_FULL_SCREEN (and make KEY_ZOOM its alias)
> - KEY_ASPECT_RATIO (and make KEY_SCREEN its alias)
> 
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> 
> Please let me know how we want merge this. Some of patches can be applied
> independently and I tried marking them as such, but some require new key
> names from input.h

Acked-by: Jiri Kosina <jkosina@suse.cz>

for the HID changes, and feel free to take it through your tree as a 
whole, I don't expect any major conflicts rising up from this.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH 1/7] Input: document meanings of KEY_SCREEN and KEY_ZOOM
From: Benjamin Tissoires @ 2019-01-21 10:41 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Dmitry Torokhov, Mauro Carvalho Chehab, open list:HID CORE LAYER,
	lkml, linux-media
In-Reply-To: <nycvar.YFH.7.76.1901211110190.6626@cbobk.fhfr.pm>

On Mon, Jan 21, 2019 at 11:11 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Fri, 18 Jan 2019, Dmitry Torokhov wrote:
>
> > It is hard to say what KEY_SCREEN and KEY_ZOOM mean, but historically DVB
> > folks have used them to indicate switch to full screen mode. Later, they
> > converged on using KEY_ZOOM to switch into full screen mode and KEY)SCREEN
> > to control aspect ratio (see Documentation/media/uapi/rc/rc-tables.rst).
> >
> > Let's commit to these uses, and define:
> >
> > - KEY_FULL_SCREEN (and make KEY_ZOOM its alias)
> > - KEY_ASPECT_RATIO (and make KEY_SCREEN its alias)
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >
> > Please let me know how we want merge this. Some of patches can be applied
> > independently and I tried marking them as such, but some require new key
> > names from input.h
>
> Acked-by: Jiri Kosina <jkosina@suse.cz>

Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

>
> for the HID changes, and feel free to take it through your tree as a
> whole, I don't expect any major conflicts rising up from this.

Works for me too. My tests showed no issues, so that's OK from me.

Cheers,
Benjamin

>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

^ permalink raw reply

* Re: [PATCH 12/13] input: max77650: add onkey support
From: Bartosz Golaszewski @ 2019-01-21 10:52 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rob Herring, Mark Rutland, Linus Walleij, Jacek Anaszewski,
	Pavel Machek, Lee Jones, Sebastian Reichel, Liam Girdwood,
	Mark Brown, Greg Kroah-Hartman, Linux Kernel Mailing List,
	open list:GPIO SUBSYSTEM, devicetree, linux-input, linux-leds,
	linux-pm, Bartosz Golaszewski
In-Reply-To: <20190119090318.GB187380@dtor-ws>

sob., 19 sty 2019 o 10:03 Dmitry Torokhov <dmitry.torokhov@gmail.com>
napisał(a):
>
> Hi Bartosz,
>
> On Fri, Jan 18, 2019 at 02:42:43PM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Add support for the push- and slide-button events for max77650.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > ---
> >  drivers/input/misc/Kconfig          |   9 ++
> >  drivers/input/misc/Makefile         |   1 +
> >  drivers/input/misc/max77650-onkey.c | 135 ++++++++++++++++++++++++++++
> >  3 files changed, 145 insertions(+)
> >  create mode 100644 drivers/input/misc/max77650-onkey.c
> >
> > diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> > index ca59a2be9bc5..bb9c45c1269e 100644
> > --- a/drivers/input/misc/Kconfig
> > +++ b/drivers/input/misc/Kconfig
> > @@ -180,6 +180,15 @@ config INPUT_M68K_BEEP
> >       tristate "M68k Beeper support"
> >       depends on M68K
> >
> > +config INPUT_MAX77650_ONKEY
> > +     tristate "Maxim MAX77650 ONKEY support"
> > +     depends on MFD_MAX77650
> > +     help
> > +       Support the ONKEY of the MAX77650 PMIC as an input device.
> > +
> > +       To compile this driver as a module, choose M here: the module
> > +       will be called max77650-onkey.
> > +
> >  config INPUT_MAX77693_HAPTIC
> >       tristate "MAXIM MAX77693/MAX77843 haptic controller support"
> >       depends on (MFD_MAX77693 || MFD_MAX77843) && PWM
> > diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
> > index 9d0f9d1ff68f..5bd53590ce60 100644
> > --- a/drivers/input/misc/Makefile
> > +++ b/drivers/input/misc/Makefile
> > @@ -43,6 +43,7 @@ obj-$(CONFIG_INPUT_IXP4XX_BEEPER)   += ixp4xx-beeper.o
> >  obj-$(CONFIG_INPUT_KEYSPAN_REMOTE)   += keyspan_remote.o
> >  obj-$(CONFIG_INPUT_KXTJ9)            += kxtj9.o
> >  obj-$(CONFIG_INPUT_M68K_BEEP)                += m68kspkr.o
> > +obj-$(CONFIG_INPUT_MAX77650_ONKEY)   += max77650-onkey.o
> >  obj-$(CONFIG_INPUT_MAX77693_HAPTIC)  += max77693-haptic.o
> >  obj-$(CONFIG_INPUT_MAX8925_ONKEY)    += max8925_onkey.o
> >  obj-$(CONFIG_INPUT_MAX8997_HAPTIC)   += max8997_haptic.o
> > diff --git a/drivers/input/misc/max77650-onkey.c b/drivers/input/misc/max77650-onkey.c
> > new file mode 100644
> > index 000000000000..cc7e83f589cd
> > --- /dev/null
> > +++ b/drivers/input/misc/max77650-onkey.c
> > @@ -0,0 +1,135 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (C) 2018 BayLibre SAS
> > + * Author: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> > + *
> > + * ONKEY driver for MAXIM 77650/77651 charger/power-supply.
> > + */
> > +
> > +#include <linux/i2c.h>
> > +#include <linux/input.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/mfd/max77650.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/regmap.h>
> > +
> > +#define MAX77650_ONKEY_MODE_MASK     BIT(3)
> > +#define MAX77650_ONKEY_MODE_PUSH     0x00
> > +#define MAX77650_ONKEY_MODE_SLIDE    BIT(3)
> > +
> > +struct max77650_onkey {
> > +     struct input_dev *input;
> > +     unsigned int code;
> > +};
> > +
> > +static irqreturn_t
> > +max77650_onkey_report(struct max77650_onkey *onkey, int value)
> > +{
> > +     input_report_key(onkey->input, onkey->code, value);
> > +     input_sync(onkey->input);
> > +
> > +     return IRQ_HANDLED;
>
> It is weird that report function returns irqreturn_t. I'd simply moved
> input_report_key()/input_sync() into real IRQ handlers.
>
> > +}
> > +
> > +static irqreturn_t max77650_onkey_falling(int irq, void *data)
> > +{
> > +     struct max77650_onkey *onkey = data;
> > +
> > +     return max77650_onkey_report(onkey, 0);
> > +}
> > +
> > +static irqreturn_t max77650_onkey_rising(int irq, void *data)
> > +{
> > +     struct max77650_onkey *onkey = data;
> > +
> > +     return max77650_onkey_report(onkey, 1);
> > +}
> > +
> > +static int max77650_onkey_probe(struct platform_device *pdev)
> > +{
> > +     struct regmap_irq_chip_data *irq_data;
> > +     struct max77650_onkey *onkey;
> > +     struct device *dev, *parent;
> > +     int irq_r, irq_f, rv, mode;
>
> Please call "rv" "error".
>
> > +     struct i2c_client *i2c;
> > +     const char *mode_prop;
> > +     struct regmap *map;
> > +
> > +     dev = &pdev->dev;
> > +     parent = dev->parent;
> > +     i2c = to_i2c_client(parent);
> > +     irq_data = i2c_get_clientdata(i2c);
> > +
> > +     map = dev_get_regmap(parent, NULL);
> > +     if (!map)
> > +             return -ENODEV;
> > +
> > +     onkey = devm_kzalloc(dev, sizeof(*onkey), GFP_KERNEL);
> > +     if (!onkey)
> > +             return -ENOMEM;
> > +
> > +     rv = device_property_read_u32(dev, "linux,code", &onkey->code);
> > +     if (rv)
> > +             onkey->code = KEY_POWER;
> > +
> > +     rv = device_property_read_string(dev, "maxim,onkey-mode", &mode_prop);
> > +     if (rv)
> > +             mode_prop = "push";
> > +
> > +     if (strcmp(mode_prop, "push") == 0)
> > +             mode = MAX77650_ONKEY_MODE_PUSH;
> > +     else if (strcmp(mode_prop, "slide") == 0)
> > +             mode = MAX77650_ONKEY_MODE_SLIDE;
> > +     else
> > +             return -EINVAL;
> > +
> > +     rv = regmap_update_bits(map, MAX77650_REG_CNFG_GLBL,
> > +                             MAX77650_ONKEY_MODE_MASK, mode);
> > +     if (rv)
> > +             return rv;
> > +
> > +     irq_f = regmap_irq_get_virq(irq_data, MAX77650_INT_nEN_F);
> > +     if (irq_f <= 0)
> > +             return -EINVAL;
> > +
> > +     irq_r = regmap_irq_get_virq(irq_data, MAX77650_INT_nEN_R);
> > +     if (irq_r <= 0)
> > +             return -EINVAL;
>
> Ugh, it would be better if you handled IRQ mapping in the MFD piece and
> passed it as resources of platform device. Then you'd simply call
> platform_get_irq() here and did not have to reach into parent device for
> "irq_dara".
>
> > +
> > +     onkey->input = devm_input_allocate_device(dev);
> > +     if (!onkey->input)
> > +             return -ENOMEM;
> > +
> > +     onkey->input->name = "max77650_onkey";
> > +     onkey->input->phys = "max77650_onkey/input0";
> > +     onkey->input->id.bustype = BUS_I2C;
> > +     onkey->input->dev.parent = dev;
>
> Not needed since devm_input_allocate_device sets parent for you.
>
> > +     input_set_capability(onkey->input, EV_KEY, onkey->code);
> > +
> > +     rv = devm_request_threaded_irq(dev, irq_f, NULL,
>
> Why threaded interrupt with only hard interrupt handler? If parent
> interrupt is threaded use "any_context_irq" here.
>

Hi Dmitry,

actually it's the other way around. Take a look at the function
prototype for  devm_request_threaded_irq()[1].

The third parameter is the hard-irq handler (NULL in my patch), the
fourth is the thread function. Actually even if I did what you're
saying - it would never work as this is a nested irq for which the
hard-irq handler is never called.

For the rest: I'll fix it in v2.

Best regards,
Bartosz Golaszewski

[1] https://elixir.bootlin.com/linux/latest/source/kernel/irq/devres.c#L52

> > +                                    max77650_onkey_falling,
> > +                                    IRQF_ONESHOT, "onkey-down", onkey);
>
> Why do you need oneshot with interrupt that is essentially not threaded?
>
> > +     if (rv)
> > +             return rv;
> > +
> > +     rv = devm_request_threaded_irq(dev, irq_r, NULL,
> > +                                    max77650_onkey_rising,
> > +                                    IRQF_ONESHOT, "onkey-up", onkey);
> > +     if (rv)
> > +             return rv;
> > +
> > +     return input_register_device(onkey->input);
> > +}
> > +
> > +static struct platform_driver max77650_onkey_driver = {
> > +     .driver = {
> > +             .name = "max77650-onkey",
> > +     },
> > +     .probe = max77650_onkey_probe,
> > +};
> > +module_platform_driver(max77650_onkey_driver);
> > +
> > +MODULE_DESCRIPTION("MAXIM 77650/77651 ONKEY driver");
> > +MODULE_AUTHOR("Bartosz Golaszewski <bgolaszewski@baylibre.com>");
> > +MODULE_LICENSE("GPL");
>
> SPDX header say GPL-2.0 so please "GPL v2" here as "GPL" means v2 and
> later.
>
> Thanks.
>
> --
> Dmitry

^ permalink raw reply

* Re: [PATCH 04/13] dt-bindings: gpio: add DT bindings for max77650
From: Linus Walleij @ 2019-01-21 14:04 UTC (permalink / raw)
  To: Bartosz Golaszewski
  Cc: Rob Herring, Mark Rutland, Dmitry Torokhov, Jacek Anaszewski,
	Pavel Machek, Lee Jones, Sebastian Reichel, Liam Girdwood,
	Mark Brown, Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
	open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Input, Linux LED Subsystem, Linux PM list,
	Bartosz Golaszewski
In-Reply-To: <20190118134244.22253-5-brgl@bgdev.pl>

On Fri, Jan 18, 2019 at 2:43 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add the DT binding document for the GPIO module of max77650.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Very simple so not much to complain about :)
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH 10/13] gpio: max77650: add GPIO support
From: Linus Walleij @ 2019-01-21 14:20 UTC (permalink / raw)
  To: Bartosz Golaszewski, Brian Masney
  Cc: Rob Herring, Mark Rutland, Dmitry Torokhov, Jacek Anaszewski,
	Pavel Machek, Lee Jones, Sebastian Reichel, Liam Girdwood,
	Mark Brown, Greg Kroah-Hartman, linux-kernel@vger.kernel.org,
	open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Input, Linux LED Subsystem, Linux PM list,
	Bartosz Golaszewski
In-Reply-To: <20190118134244.22253-11-brgl@bgdev.pl>

Hi Bartosz,

thanks for the patch!

On Fri, Jan 18, 2019 at 2:43 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:

> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Add GPIO support for max77650 mfd device. This PMIC exposes a single
> GPIO line.
>
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>

Overall you know for sure what you're doing so not much to
say about the GPIO chip etc. The .set_config() is nice and
helpful (maybe you will be able to also add pull up/down
using Thomas Petazzoni's new config interfaces!)

However enlighten me on this:

> +static int max77650_gpio_to_irq(struct gpio_chip *gc, unsigned int offset)
> +{
> +       struct max77650_gpio_chip *chip = gpiochip_get_data(gc);
> +
> +       return regmap_irq_get_virq(chip->irq_chip_data, MAX77650_INT_GPI);
> +}

I know this may be opening the gates to a lot of coding, but
isn't this IRQ hierarchical? I.e. that irqchip is not in the
node of the GPIO chip but in the node of the MFD top
device, with a 1:1 mapping between some of the IRQs
and a certain GPIO line.

Using regmap IRQ makes it confusing for me so I do not
know for sure if that is the case.

I am worried that you are recreating a problem (present in many
drivers, including some written by me, mea culpa) that Brian Masney
has been trying to solve for the gpiochip inside the SPMI
GPIO (drivers/pinctrl/qcom/pinctrl-spmi-gpio.c).

I have queued Brians refactoring and solution here:
https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-qcom-spmi

And the overall description on what he's trying to achieve is
here:
https://marc.info/?l=linux-gpio&m=154793071511130&w=2

My problem description (which I fear will apply also to this
driver):
https://www.spinics.net/lists/linux-gpio/msg34655.html

I plan to merge Brians patches soon-ish to devel and then from
there try to construct more helpers in the gpiolib core,
and if possible fix some of the old drivers who rely on
.to_irq().

We will certainly fix ssbi-gpio as well, and that is a good start
since the Qualdcomm platforms are so pervasive in the
market.

But maybe this doesn't apply here? I am not the smartest...
Just want to make sure that it is possible to refer an
interrupt directly to this DT node, as it is indeed marked
as interrupt-controller.

Yours,
Linus Walleij

^ permalink raw reply

* Re: [PATCH v3 1/4] dt-bindings: input: touchscreen: goodix: Document AVDD28-supply property
From: Rob Herring @ 2019-01-21 16:16 UTC (permalink / raw)
  To: Jagan Teki
  Cc: devicetree, Dmitry Torokhov, Chen-Yu Tsai, linux-input,
	linux-kernel@vger.kernel.org, Michael Trimarchi, linux-amarula
In-Reply-To: <CAMty3ZCyq=tZMt6RC1aYJuEWgYp2zv1LBh+ZUwucE+g4RCoUaQ@mail.gmail.com>

On Fri, Jan 18, 2019 at 10:01 AM Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> On Wed, Jan 9, 2019 at 7:08 PM Rob Herring <robh@kernel.org> wrote:
> >
> > Please CC DT list if you want bindings reviewed.
>
> Sorry I forgot.
>
> >
> > On Wed, Jan 9, 2019 at 1:40 AM Dmitry Torokhov
> > <dmitry.torokhov@gmail.com> wrote:
> > >
> > > On Sat, Dec 15, 2018 at 08:47:59PM +0530, Jagan Teki wrote:
> > > > Most of the Goodix CTP controllers are supply with AVDD28 pin.
> > > > which need to supply for controllers like GT5663 on some boards
> > > > to trigger the power.
> > > >
> > > > So, document the supply property so-that the require boards
> > > > that used on GT5663 can enable it via device tree.
> > > >
> > > > Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> > > > index f7e95c52f3c7..c4622c983e08 100644
> > > > --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> > > > +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> > > > @@ -23,6 +23,7 @@ Optional properties:
> > > >   - touchscreen-inverted-y  : Y axis is inverted (boolean)
> > > >   - touchscreen-swapped-x-y : X and Y axis are swapped (boolean)
> > > >                               (swapping is done after inverting the axis)
> > > > + - AVDD28-supply     : Analog power supply regulator on AVDD28 pin
> > >
> > > I think we normally use lower case in DT bindings and rarely encode
> > > voltage in the supply name unless we are dealing with several supplies
> > > of the same kind, but I'll let Ron comment on this.
> >
> > Yes on lowercase though there are some exceptions.
> >
> > There's also a AVDD22 supply as well as DVDD12 and VDDIO. So we
> > probably need to keep the voltage, but the binding is incomplete.
>
> What is incomplete here? can you please elaborate.

You are missing the 3 other supplies the chip has: AVDD22, DVDD12 and VDDIO.

Rob

^ permalink raw reply

* Re: [PATCH 10/13] gpio: max77650: add GPIO support
From: Bartosz Golaszewski @ 2019-01-21 17:07 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Brian Masney, Rob Herring, Mark Rutland, Dmitry Torokhov,
	Jacek Anaszewski, Pavel Machek, Lee Jones, Sebastian Reichel,
	Liam Girdwood, Mark Brown, Greg Kroah-Hartman,
	linux-kernel@vger.kernel.org, open list:GPIO SUBSYSTEM,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux Input, Linux LED Subsystem, Linux PM list
In-Reply-To: <CACRpkdZhy7yBZrGLxhRGaac7v5jZhgUSz9gZAhFXbevJbKjpZw@mail.gmail.com>

pon., 21 sty 2019 o 15:20 Linus Walleij <linus.walleij@linaro.org> napisał(a):
>
> Hi Bartosz,
>
> thanks for the patch!
>
> On Fri, Jan 18, 2019 at 2:43 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> > From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
> >
> > Add GPIO support for max77650 mfd device. This PMIC exposes a single
> > GPIO line.
> >
> > Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>
> Overall you know for sure what you're doing so not much to
> say about the GPIO chip etc. The .set_config() is nice and
> helpful (maybe you will be able to also add pull up/down
> using Thomas Petazzoni's new config interfaces!)
>
> However enlighten me on this:
>
> > +static int max77650_gpio_to_irq(struct gpio_chip *gc, unsigned int offset)
> > +{
> > +       struct max77650_gpio_chip *chip = gpiochip_get_data(gc);
> > +
> > +       return regmap_irq_get_virq(chip->irq_chip_data, MAX77650_INT_GPI);
> > +}
>
> I know this may be opening the gates to a lot of coding, but
> isn't this IRQ hierarchical? I.e. that irqchip is not in the
> node of the GPIO chip but in the node of the MFD top
> device, with a 1:1 mapping between some of the IRQs
> and a certain GPIO line.
>
> Using regmap IRQ makes it confusing for me so I do not
> know for sure if that is the case.
>
> I am worried that you are recreating a problem (present in many
> drivers, including some written by me, mea culpa) that Brian Masney
> has been trying to solve for the gpiochip inside the SPMI
> GPIO (drivers/pinctrl/qcom/pinctrl-spmi-gpio.c).
>
> I have queued Brians refactoring and solution here:
> https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git/log/?h=ib-qcom-spmi
>
> And the overall description on what he's trying to achieve is
> here:
> https://marc.info/?l=linux-gpio&m=154793071511130&w=2
>
> My problem description (which I fear will apply also to this
> driver):
> https://www.spinics.net/lists/linux-gpio/msg34655.html
>
> I plan to merge Brians patches soon-ish to devel and then from
> there try to construct more helpers in the gpiolib core,
> and if possible fix some of the old drivers who rely on
> .to_irq().
>
> We will certainly fix ssbi-gpio as well, and that is a good start
> since the Qualdcomm platforms are so pervasive in the
> market.
>
> But maybe this doesn't apply here? I am not the smartest...
> Just want to make sure that it is possible to refer an
> interrupt directly to this DT node, as it is indeed marked
> as interrupt-controller.
>

Hi Linus,

Thank you for your review. While I think you're right about the issue
being present in this driver, I'm not sure it's really a problem. Do
we actually require every gpio-controller to also be a stand-alone
interrupt-controller? The binding document for the GPIO module doesn't
mention this - it only requires the gpio-controller property. Without
the "interrupt-controller" property dtc will bail-out if anyone uses
this node as the interrupt parent.

If I'm wrong and we do require it, then I think we need to update
Documentation/devicetree/bindings/gpio/gpio.txt.

Best regards,
Bartosz Golaszewski

PS: FYI since this submission I dropped the virtual irq number lookup
in sub-drivers in favor of resources setup by the parent driver[1] as
suggested by Dmitry in his review of the input module driver.

[1] https://github.com/brgl/linux/blob/topic/max77650_mfd_driver/drivers/gpio/gpio-max77650.c#L158

^ permalink raw reply

* Re: [PATCH] Input: synaptics - add PNP IDs for Dell XPS models to forcepad
From: Kim Phillips @ 2019-01-21 23:10 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Dmitry Torokhov, lkml, Paul Menzel, open list:HID CORE LAYER
In-Reply-To: <CAO-hwJLBd-VGAoMBpwEZhWY2khsDEz2qKh05tNjbNVhAzQCRJA@mail.gmail.com>

On 1/17/19 2:13 AM, Benjamin Tissoires wrote:
> On Wed, Jan 16, 2019 at 5:59 AM Kim Phillips <kim@kimphillips.com> wrote:
>>
>> On 1/15/19 2:57 AM, Benjamin Tissoires wrote:
>>> On Mon, Jan 14, 2019 at 7:40 PM Dmitry Torokhov
>>> <dmitry.torokhov@gmail.com> wrote:
>>>>
>>>> On Sat, Jan 12, 2019 at 04:04:36PM -0600, Kim Phillips wrote:
>>>>> On 1/11/19 7:40 PM, Dmitry Torokhov wrote:
>>>>>> Hi Kim,
>>>>>
>>>>> Hi Dmitry,
>>>>>
>>>>>> On Fri, Jan 11, 2019 at 02:54:30PM -0600, Kim Phillips wrote:
>>>>>>> This patch is the result of seeing this message:
>>>>>>>
>>>>>>> psmouse serio1: synaptics: Your touchpad (PNP: DLL087c 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.
>>>>>>>
>>>>>>> If I set psmouse.synaptics_intertouch=1, or add the PNP ID to
>>>>>>> smbus_pnp_ids, the touchpad continues to work, and the above message
>>>>>>> goes away, but we now get:
>>>>>>>
>>>>>>> psmouse serio1: synaptics: Trying to set up SMBus access
>>>>>>> psmouse serio1: synaptics: SMbus companion is not ready yet
>>>>>>>
>>>>>>> With this patch applied, i.e., the PNP IDs are added to the forcepad
>>>>>>> array, the touchpad continues to work and all of the above messages
>>>>>>> disappear.
>>>>>>
>>>>>> Are you sure the touchpad in XPSes is a forcepad (i.e. it does not have
>>>>>> physical button underneath it)? As far as I know there were only couple
>>>>>> of HP laptops with forcepads and when switching to RMI mode forcepads
>>>>>> need F21 handler that we do not currently have in the kernel.
>>>>>
>>>>> I see, no, I'm not sure, but assuming you're right, the IDs
>>>>> should be added to the smbus array instead, after fixing
>>>>> the SMbus "companion not ready" problem?  Pointers for that and
>>>>> the below interrupts when touchpad idle after resume, welcome.
>>>>>
>>>>> Also, the link to get the RMI4 spec in
>>>>> Documentation/devicetree/bindings/input/rmi4/rmi_2d_sensor.txt
>>>>> is broken.  Any pointers for that also appreciated.
>>>>
>>>> OK, sorting it all out some more:
>>>>
>>>> - because we do not have support for F21 necessary for forcepads adding
>>>>     APIC ID to forcepad list actuallty disables SMbus companion mode, that
>>>>     is why you no longer see "companion not ready" messages vs. setting
>>>>     psmouse.synaptics_intertouch=1 module parameter.
>>>
>>> Yep
>>>
>>>>
>>>> - this does not really matter as your touchpad ends up being driven by
>>>>     i2c-hid and hid-multitouch drivers, and that is how we wait it to
>>>>     work, as we do not want to deviate from behavior on Windows since OEM
>>>>     tested it (the device and firmware) in tha configuration.
>>>
>>> Yep too. The I2C-hid touchpads from Synaptics do not have the SMBus
>>> wired at all, so we can't enable SMBus for them. Also, the fact that
>>> the device gets loaded over i2c-hid means that we can't know that it
>>> is the case from the psmouse/synaptics point of view.
>>
>> Sigh, OK, I wasn't registering the word "not" when reading
>> "If i2c-hid and hid-rmi are not used" too quickly.
>>
>>>> - we need to figure out issue with interrupts on resume, maybe Benjamin
>>>>     have seen it?
>>>
>>> First time I see it.
>>>
>>> I just tried on a XPS 9360 and kernel v4.18 (fedora) and nothing was a problem.
>>> I then tried on a XPS 9575 with v4.19, and here, the wacom I2C node is
>>> also keeping firing the interrupts, but not the touchpad. However,
>>> here, the interrupts are not stopping when I touch the touchscreen or
>>> if I approach a pen.
>>>
>>> Kim, rmmod-ing i2c-hid and modprobing back it with the parameter
>>> debug=1 doesn't show any events processed when the interrupts are
>>> firing. Do you see the same?
>>
>> After rmmodding i2c_hid, the WCOM488F and SYNA2393 entries in /proc/interrupts
>> somewhat expectedly disappear, however, the modprobe (with or without debug=1)
>> fails to bring them back, with these messages left in dmesg:
>>
>> [ 3882.073222] calling  i2c_hid_driver_init+0x0/0x1000 [i2c_hid] @ 3082
>> [ 3882.180938] i2c_hid i2c-WCOM488F:00: HID over i2c has not been provided an Int IRQ
>> [ 3882.181060] i2c_hid: probe of i2c-WCOM488F:00 failed with error -22
>> [ 3882.181065] probe of i2c-WCOM488F:00 returned 0 after 496 usecs
>> [ 3882.289196] i2c_hid i2c-SYNA2393:00: HID over i2c has not been provided an Int IRQ
>> [ 3882.289318] i2c_hid: probe of i2c-SYNA2393:00 failed with error -22
>> [ 3882.289321] probe of i2c-SYNA2393:00 returned 0 after 508 usecs
>> [ 3882.289418] initcall i2c_hid_driver_init+0x0/0x1000 [i2c_hid] returned 0 after 211119 usecs
> 
> Yes, that is a weird behavior I experience the other day also. It
> looks like rmmod-ing the driver simply kills the irq description
> attached to the i2c node. This waasn't the case previously, and I
> wonder if the issue is in i2c-hid (unlikely) or in i2c-core.
> 
>>
>> In order to work around that problem, I set h2c_hid.debug=1 in the
>> boot command line, and after a resume, dmesg contains these messages,
>> relevant to i2c_hid:
>>
>> [  267.235673] i2c_hid i2c-WCOM488F:00: calling i2c_hid_resume+0x0/0x140 [i2c_hid] @ 3078, parent: i2c-9
>> [  267.235676] input input16: calling input_dev_resume+0x0/0x50 @ 3060, parent: card0
>> [  267.235681] input input16: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
>> [  267.235682] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power
>> [  267.235687] input input17: calling input_dev_resume+0x0/0x50 @ 3060, parent: card0
>> [  267.235689] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 00 08
>> [  267.235693] input input17: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
>> [  267.235698] idma64 idma64.1: calling platform_pm_resume+0x0/0x50 @ 3060, parent: 0000:00:15.1
>> [  267.235701] idma64 idma64.1: platform_pm_resume+0x0/0x50 returned 0 after 0 usecs
>> [  267.235706] i2c_designware i2c_designware.1: calling platform_pm_resume+0x0/0x50 @ 3060, parent: 0000:00:15.1
>> [  267.235709] i2c_designware i2c_designware.1: platform_pm_resume+0x0/0x50 returned 0 after 0 usecs
>> [  267.235718] rfkill rfkill0: calling rfkill_resume+0x0/0x60 @ 3060, parent: phy0
>> [  267.235724] rfkill rfkill0: rfkill_resume+0x0/0x60 returned 0 after 3 usecs
>> [  267.235768] i2c_hid i2c-SYNA2393:00: calling i2c_hid_resume+0x0/0x140 [i2c_hid] @ 3108, parent: i2c-10
>> [  267.235774] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power
>> [  267.235776] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 00 08
>> [  267.236051] i2c_hid i2c-SYNA2393:00: i2c_hid_set_or_send_report
>> [  267.236053] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 3d 03 23 00 04 00 0d 00
>> [  267.236076] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [  267.236080] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 fd 89 a3 07 f3 02 20 8d 49 06 d7 9c ff ff 80 59 5c 13 d7 9c ff ff 08 00 00 08 d7 9c ff ff 01 00 00 00 00 00 00 00 16 00 00 00 66 69 6c 65 75 74 6c 2e 6d 65 73 73
>> [  267.237691] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.238137] i2c_hid i2c-SYNA2393:00: i2c_hid_set_or_send_report
>> [  267.238140] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 34 03 23 00 04 00 04 03
>> [  267.238670] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
>> [  267.238673] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
>> [  267.239708] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.240084] i2c_hid i2c-SYNA2393:00: i2c_hid_set_or_send_report
>> [  267.240087] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 36 03 23 00 04 00 06 03
>> [  267.241377] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [  267.241380] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> [  267.241711] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.243279] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.243981] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
>> [  267.243984] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
>> [  267.244847] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.245360] i2c_hid i2c-SYNA2393:00: i2c_hid_resume+0x0/0x140 [i2c_hid] returned 0 after 9360 usecs
>> [  267.245455] input input27: calling input_dev_resume+0x0/0x50 @ 3060, parent: 0018:06CB:7A13.0002
>> [  267.245460] input input27: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
>> [  267.246773] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [  267.246777] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> [  267.246919] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.248517] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.249383] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
>> [  267.249386] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
>> [  267.250116] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.251767] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.252137] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [  267.252140] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> [  267.253440] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.254734] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
>> [  267.254737] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
>> [  267.255015] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.256612] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.257475] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [  267.257478] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> [  267.258275] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.259852] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.260046] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
>> [  267.260049] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
>> [  267.261441] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.262789] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [  267.262792] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> [  267.263129] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.264718] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.265410] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
>> [  267.265413] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
>> [  267.266318] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.267971] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.268169] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
>> [  267.268172] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3e 03 05 00 05 00 0e 02 00
>> [  267.268728] i2c_hid i2c-WCOM488F:00: i2c_hid_resume+0x0/0x140 [i2c_hid] returned 0 after 32273 usecs
>> [  267.269575] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.271144] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.272719] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>> [  267.274298] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 3c 03 a8 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 60 01 00
>>
>> ...and it goes on and on, where that same last line gets
>> repeated as the interrupts fire, until I start using the touchpad,
>> thereby stopping the interrupts.
> 
> It looks like valid inputs. So I would expect the firmware to unset
> the IRQ line, but it doesn't do it.
> 
> On an other thread, it has been brought to my attention that maybe we
> are not 100% compatible with the Windows driver regarding
> sleep/power_on commands. We expect the device to be ready while the
> Windows driver waits a little before sending a command. Maybe we can
> try to see if adding I2C_HID_QUIRK_DELAY_AFTER_SLEEP to this device
> changes something.

Thought I'd try DELAY_AFTER_SLEEP.  Please double check whether
I did that correctly given these diagnostics:

[    5.720289] input: SYNA2393:00 06CB:7A13 Touchpad as /devices/pci0000:00/0000:00:15.1/i2c_designware.1/i2c-10/i2c-SYNA2393:00/0018:06CB:7A13.0006/input/input20
[    5.720362] hid-multitouch 0018:06CB:7A13.0006: input,hidraw5: I2C HID v1.00 Mouse [SYNA2393:00 06CB:7A13] on i2c-SYNA2393:00

I added both 2393 and 7A13 as model numbers just
because I wasn't sure which one was which:

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 27519eb8ee63..d297ad6744e2 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1072,6 +1072,9 @@
  #define USB_DEVICE_ID_SYNAPTICS_HD     0x0ac3
  #define USB_DEVICE_ID_SYNAPTICS_QUAD_HD        0x1ac3
  #define USB_DEVICE_ID_SYNAPTICS_TP_V103        0x5710
+#define I2C_VENDOR_ID_SYNAPTICS                0x06cb
+#define I2C_PRODUCT_ID_SYNAPTICS_2393  0x2393
+#define I2C_PRODUCT_ID_SYNAPTICS_7A13  0x7a13
  
  #define USB_VENDOR_ID_TEXAS_INSTRUMENTS        0x2047
  #define USB_DEVICE_ID_TEXAS_INSTRUMENTS_LENOVO_YOGA    0x0855
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 8555ce7e737b..cfdd5f2e4781 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -179,6 +179,10 @@ static const struct i2c_hid_quirks {
                 I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
         { USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
                 I2C_HID_QUIRK_NO_RUNTIME_PM },
+       { I2C_VENDOR_ID_SYNAPTICS, I2C_PRODUCT_ID_SYNAPTICS_2393,
+               I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
+       { I2C_VENDOR_ID_SYNAPTICS, I2C_PRODUCT_ID_SYNAPTICS_7A13,
+               I2C_HID_QUIRK_DELAY_AFTER_SLEEP },
         { 0, 0 }
  };
  
@@ -419,14 +423,19 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state)
                         delay = jiffies_to_usecs(ihid->sleep_delay - now);
                         usleep_range(delay, delay + 1);
                 }
-       }
+               dev_err(&client->dev, "%s %d: delayed after sleep with power on\n", __func__, __LINE__);
+       } else
+               dev_err(&client->dev, "%s %d: did NOT delay after sleep whilst powered on. No quirk?\n", __func__, __LINE__);
  
         ret = __i2c_hid_command(client, &hid_set_power_cmd, power_state,
                 0, NULL, 0, NULL, 0);
  
         if (ihid->quirks & I2C_HID_QUIRK_DELAY_AFTER_SLEEP &&
-           power_state == I2C_HID_PWR_SLEEP)
+           power_state == I2C_HID_PWR_SLEEP) {
                 ihid->sleep_delay = jiffies + msecs_to_jiffies(20);
+               dev_err(&client->dev, "%s %d: delayed after sleep with power set to sleep\n", __func__, __LINE__);
+       } else
+               dev_err(&client->dev, "%s %d: did NOT delay after sleep at sleep. No quirk?\n", __func__, __LINE__);
  
         if (ret)
                 dev_err(&client->dev, "failed to change power setting.\n");


This is as I give the suspend command, using the touchpad to
click on the pause button in Gnome (whilst holding down ALT):

[  319.326944] i2c_hid i2c-SYNA2393:00: input: 20 00 03 03 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 e2 c4 01 00
[  319.334357] i2c_hid i2c-SYNA2393:00: input: 20 00 03 03 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 2b c5 01 00
[  319.341692] i2c_hid i2c-SYNA2393:00: input: 20 00 03 03 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 74 c5 01 00
[  319.348994] i2c_hid i2c-SYNA2393:00: input: 20 00 03 03 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 bd c5 01 00
[  319.355556] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
...
[  349.858808] PM: suspend entry (deep)
[  349.858810] PM: Syncing filesystems ... done.
...
[  349.985145] i2c_hid i2c-SYNA2393:00: calling acpi_subsys_suspend+0x0/0x60 @ 7, parent: i2c-10
[  349.985147] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power
[  349.985149] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power 428: did NOT delay after sleep whilst powered on. No quirk?
[  349.985187] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 01 08
[  349.985555] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power 436: delayed after sleep with power set to sleep
[  349.985591] i2c_hid i2c-SYNA2393:00: acpi_subsys_suspend+0x0/0x60 returned 0 after 431 usecs
[  349.985636] input input17: calling input_dev_suspend+0x0/0x60 @ 12762, parent: card0
[  349.985639] input input17: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[  349.985642] input input16: calling input_dev_suspend+0x0/0x60 @ 12762, parent: card0
[  349.985645] input input16: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[  349.985648] input input15: calling input_dev_suspend+0x0/0x60 @ 12762, parent: card0
[  349.985650] input input15: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[  349.985653] input input14: calling input_dev_suspend+0x0/0x60 @ 12762, parent: card0
[  349.985656] input input14: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[  349.985659] input input13: calling input_dev_suspend+0x0/0x60 @ 12762, parent: card0
[  349.985661] input input13: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[  349.985664] input input12: calling input_dev_suspend+0x0/0x60 @ 12762, parent: card0
[  349.985667] input input12: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[  349.985678] i2c_designware i2c_designware.1: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: 0000:00:15.1
[  349.985680] i2c_designware i2c_designware.1: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[  349.985684] idma64 idma64.1: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: 0000:00:15.1
[  349.985695] idma64 idma64.1: platform_pm_suspend+0x0/0x50 returned 0 after 9 usecs
[  349.985698] leds dell::kbd_backlight: calling led_suspend+0x0/0x30 @ 12762, parent: dell-laptop
[  349.985700] leds dell::kbd_backlight: led_suspend+0x0/0x30 returned 0 after 0 usecs
[  349.985711] coretemp coretemp.0: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: platform
[  349.985713] coretemp coretemp.0: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[  349.985717] snd_hda_codec_hdmi hdaudioC0D2: calling pm_runtime_force_suspend+0x0/0xe0 @ 7, parent: 0000:00:1f.3
[  349.985720] snd_hda_codec_hdmi hdaudioC0D2: pm_runtime_force_suspend+0x0/0xe0 returned 0 after 0 usecs
[  349.985727] i2c_hid i2c-WCOM488F:00: calling acpi_subsys_suspend+0x0/0x60 @ 7, parent: i2c-9
[  349.985729] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power
[  349.985730] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power 428: did NOT delay after sleep whilst powered on. No quirk?
[  349.985751] snd_hda_codec_realtek hdaudioC0D0: calling pm_runtime_force_suspend+0x0/0xe0 @ 311, parent: 0000:00:1f.3
[  349.985765] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 01 08
[  349.986162] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power 438: did NOT delay after sleep at sleep. No quirk?
[  349.986199] i2c_hid i2c-WCOM488F:00: acpi_subsys_suspend+0x0/0x60 returned 0 after 457 usecs
[  349.986250] input input11: calling input_dev_suspend+0x0/0x60 @ 12762, parent: 9DBB5994-A997-11DA-B012-B622A1EF5492
[  349.986253] input input11: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[  349.986257] platform regulatory.0: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: platform
[  349.986259] platform regulatory.0: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[  349.986262] dell-laptop dell-laptop: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: platform
[  349.986264] dell-laptop dell-laptop: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[  349.986267] dell-smbios dell-smbios.1: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: platform
[  349.986269] dell-smbios dell-smbios.1: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[  349.986272] dell-smbios dell-smbios.0: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: platform
[  349.986274] dell-smbios dell-smbios.0: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[  349.986280] dcdbas dcdbas: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: platform
[  349.986282] dcdbas dcdbas: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[  349.986293] leds input4::scrolllock: calling led_suspend+0x0/0x30 @ 12762, parent: input4
[  349.986295] leds input4::scrolllock: led_suspend+0x0/0x30 returned 0 after 0 usecs
[  349.986297] leds input4::capslock: calling led_suspend+0x0/0x30 @ 12762, parent: input4
[  349.986299] leds input4::capslock: led_suspend+0x0/0x30 returned 0 after 0 usecs
[  349.986301] leds input4::numlock: calling led_suspend+0x0/0x30 @ 12762, parent: input4
[  349.986303] leds input4::numlock: led_suspend+0x0/0x30 returned 0 after 0 usecs
[  349.986306] i2c_designware i2c_designware.0: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: 0000:00:15.0
[  349.986308] i2c_designware i2c_designware.0: platform_pm_suspend+0x0/0x50 returned 0 after 0 usecs
[  349.986312] idma64 idma64.0: calling platform_pm_suspend+0x0/0x50 @ 12762, parent: 0000:00:15.0
[  349.986322] idma64 idma64.0: platform_pm_suspend+0x0/0x50 returned 0 after 8 usecs
[  349.986329] input input10: calling input_dev_suspend+0x0/0x60 @ 12762, parent: INT33D5:00
[  349.986332] input input10: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[  349.986335] input input9: calling input_dev_suspend+0x0/0x60 @ 12762, parent: INT33D5:00
[  349.986337] input input9: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[  349.986347] input input6: calling input_dev_suspend+0x0/0x60 @ 12762, parent: serio1
[  349.986349] input input6: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[  349.986357] input input8: calling input_dev_suspend+0x0/0x60 @ 12762, parent: LNXVIDEO:01
[  349.986360] input input8: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
[  349.986363] input input7: calling input_dev_suspend+0x0/0x60 @ 12762, parent: LNXVIDEO:00
[  349.986365] input input7: input_dev_suspend+0x0/0x60 returned 0 after 0 usecs
...
[  349.987136] psmouse serio1: calling serio_suspend+0x0/0x20 @ 12762, parent: i8042
...
[  351.598355] i2c_hid i2c-SYNA2393:00: calling acpi_subsys_suspend_late+0x0/0x60 @ 12784, parent: i2c-10
[  351.598366] i2c_hid i2c-SYNA2393:00: acpi_subsys_suspend_late+0x0/0x60 returned 0 after 1 usecs
[  351.598465] i2c_designware i2c_designware.1: calling dw_i2c_plat_suspend+0x0/0x40 @ 12762, parent: 0000:00:15.1
[  351.598487] i2c_designware i2c_designware.1: dw_i2c_plat_suspend+0x0/0x40 returned 0 after 13 usecs
[  351.598529] i2c_hid i2c-WCOM488F:00: calling acpi_subsys_suspend_late+0x0/0x60 @ 87, parent: i2c-9
[  351.598540] i2c_hid i2c-WCOM488F:00: acpi_subsys_suspend_late+0x0/0x60 returned 0 after 1 usecs
[  351.598609] i2c_designware i2c_designware.0: calling dw_i2c_plat_suspend+0x0/0x40 @ 12762, parent: 0000:00:15.0
[  351.598628] i2c_designware i2c_designware.0: dw_i2c_plat_suspend+0x0/0x40 returned 0 after 12 usecs
...
[  351.633232] i2c_hid i2c-SYNA2393:00: calling acpi_subsys_suspend_noirq+0x0/0x70 @ 12784, parent: i2c-10
[  351.633235] i2c_hid i2c-SYNA2393:00: acpi_subsys_suspend_noirq+0x0/0x70 returned 0 after 0 usecs
[  351.633258] i2c_hid i2c-WCOM488F:00: calling acpi_subsys_suspend_noirq+0x0/0x70 @ 12783, parent: i2c-9
[  351.633261] i2c_hid i2c-WCOM488F:00: acpi_subsys_suspend_noirq+0x0/0x70 returned 0 after 0 usecs

Here is the relevant portion of the resume cycle,
which is similar to the one I posted earlier:

[  353.256580] leds dell::kbd_backlight: led_resume+0x0/0x30 returned 0 after 0 usecs
[  353.256585] idma64 idma64.1: calling platform_pm_resume+0x0/0x50 @ 12762, parent: 0000:00:15.1
[  353.256589] idma64 idma64.1: platform_pm_resume+0x0/0x50 returned 0 after 0 usecs
[  353.256594] i2c_designware i2c_designware.1: calling platform_pm_resume+0x0/0x50 @ 12762, parent: 0000:00:15.1
[  353.256597] i2c_designware i2c_designware.1: platform_pm_resume+0x0/0x50 returned 0 after 0 usecs
[  353.256606] input input12: calling input_dev_resume+0x0/0x50 @ 12762, parent: card0
[  353.256611] input input12: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[  353.256616] input input13: calling input_dev_resume+0x0/0x50 @ 12762, parent: card0
[  353.256625] i2c_hid i2c-WCOM488F:00: calling i2c_hid_resume+0x0/0x140 [i2c_hid] @ 12809, parent: i2c-9
[  353.256630] input input13: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[  353.256632] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power
[  353.256635] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power 428: did NOT delay after sleep whilst powered on. No quirk?
[  353.256639] input input14: calling input_dev_resume+0x0/0x50 @ 12762, parent: card0
[  353.256645] input input14: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[  353.256708] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 00 08
[  353.256712] input input15: calling input_dev_resume+0x0/0x50 @ 12762, parent: card0
[  353.256717] input input15: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[  353.256722] input input16: calling input_dev_resume+0x0/0x50 @ 12762, parent: card0
[  353.256727] input input16: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[  353.256732] input input17: calling input_dev_resume+0x0/0x50 @ 12762, parent: card0
[  353.256736] input input17: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[  353.256759] i2c_hid i2c-SYNA2393:00: calling i2c_hid_resume+0x0/0x140 [i2c_hid] @ 12791, parent: i2c-10
[  353.256767] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power
[  353.256771] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power 426: delayed after sleep with power on
[  353.256833] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 00 08
[  353.257109] i2c_hid i2c-WCOM488F:00: i2c_hid_set_power 438: did NOT delay after sleep at sleep. No quirk?
[  353.257135] i2c_hid i2c-SYNA2393:00: i2c_hid_set_power 438: did NOT delay after sleep at sleep. No quirk?
[  353.257176] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[  353.257233] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 1c 1c 15 40 25 ac 00 10 ba 13 17 be ff ff 02 00 00 00 00 00 00 00 38 07 00 7e a3 94 ff ff 18 0f 00 7e a3 94 ff ff d8 29 ca 75 a3 94 ff ff b0 07 00 7e a3 94 ff ff
[  353.257240] i2c_hid i2c-SYNA2393:00: i2c_hid_set_or_send_report
[  353.257244] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 3d 03 23 00 04 00 0d 00
[  353.258737] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.259889] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
[  353.259892] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
[  353.260345] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.261936] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.262416] i2c_hid i2c-SYNA2393:00: i2c_hid_set_or_send_report
[  353.262420] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 34 03 23 00 04 00 04 03
[  353.262646] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[  353.262650] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  353.264051] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.265306] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
[  353.265309] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
[  353.265631] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.267232] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.267705] i2c_hid i2c-SYNA2393:00: i2c_hid_set_or_send_report
[  353.267708] i2c_hid i2c-SYNA2393:00: __i2c_hid_command: cmd=22 00 36 03 23 00 04 00 06 03
[  353.268076] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[  353.268080] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  353.269304] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.270683] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
[  353.270687] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
[  353.270877] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.272471] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.272983] i2c_hid i2c-SYNA2393:00: i2c_hid_resume+0x0/0x140 [i2c_hid] returned 0 after 15834 usecs
[  353.273091] input input27: calling input_dev_resume+0x0/0x50 @ 12762, parent: 0018:06CB:7A13.0002
[  353.273096] input input27: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[  353.273104] rfkill rfkill0: calling rfkill_resume+0x0/0x60 @ 12762, parent: phy0
[  353.273112] rfkill rfkill0: rfkill_resume+0x0/0x60 returned 0 after 4 usecs
[  353.273121] input input29: calling input_dev_resume+0x0/0x50 @ 12762, parent: 1-12:1.0
[  353.273125] input input29: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[  353.273130] rfkill rfkill1: calling rfkill_resume+0x0/0x60 @ 12762, parent: hci0
[  353.273134] rfkill rfkill1: rfkill_resume+0x0/0x60 returned 0 after 0 usecs
[  353.273436] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[  353.273439] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  353.274593] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.276065] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
[  353.276069] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
[  353.276212] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.277809] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.278839] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[  353.278843] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.281024] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.281449] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
[  353.281453] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
[  353.282614] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.284248] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[  353.284251] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 03 05 00 42 00 0b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  353.284326] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.285890] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.286847] i2c_hid i2c-WCOM488F:00: i2c_hid_get_report
[  353.286850] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3b 02 05 00
[  353.287489] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.289099] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.289595] i2c_hid i2c-WCOM488F:00: i2c_hid_set_or_send_report
[  353.289598] i2c_hid i2c-WCOM488F:00: __i2c_hid_command: cmd=04 00 3e 03 05 00 05 00 0e 02 00
[  353.290178] i2c_hid i2c-WCOM488F:00: i2c_hid_resume+0x0/0x140 [i2c_hid] returned 0 after 32758 usecs
[  353.290275] input input30: calling input_dev_resume+0x0/0x50 @ 12762, parent: 0018:056A:488F.0001
[  353.290281] input input30: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[  353.290287] input input31: calling input_dev_resume+0x0/0x50 @ 12762, parent: 0018:056A:488F.0001
[  353.290292] input input31: input_dev_resume+0x0/0x50 returned 0 after 0 usecs
[  353.290804] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.291066] acpi LNXPOWER:07: Turning OFF
[  353.292115] acpi LNXPOWER:02: Turning OFF
[  353.292403] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.293924] acpi LNXPOWER:01: Turning OFF
[  353.294004] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.295015] OOM killer enabled.
[  353.295017] Restarting tasks ...
[  353.295617] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.297197] done.
[  353.297238] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.298871] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.300494] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.302107] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.303732] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.305342] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.306955] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
[  353.308955] i2c_hid i2c-SYNA2393:00: input: 20 00 03 01 38 02 a5 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 06 c6 01 00
...and so on until I touch the touchpad...

Note that, as without DELAY_AFTER_SLEEP, the last repeated
input values above after resume equal those in the last input
prior to suspend, so it's as if the touchpad's last interrupt
didn't get handled/acknowledged, and the contents of buffer
survived the suspend-resume cycle?  I can't really tell what
those values mean - asked Dell for the manual for the touchpad
and got told they don't support Linux :(

Kim

^ permalink raw reply related

* [PATCH] input: keyboard: gpio-keys-polled: use input name from pdata if available
From: Enrico Weigelt, metux IT consult @ 2019-01-22 12:00 UTC (permalink / raw)
  To: juhosg, nunojpg, linux-input, linux-gpio, linux-kernel

Instead of hardcoding the input name to the driver name ('gpio-keys-polled'),
allow the passing a name via platform data ('name' field was already present),
but default to old behaviour in case of NULL.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/input/keyboard/gpio_keys_polled.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/keyboard/gpio_keys_polled.c b/drivers/input/keyboard/gpio_keys_polled.c
index edc7262..3312186 100644
--- a/drivers/input/keyboard/gpio_keys_polled.c
+++ b/drivers/input/keyboard/gpio_keys_polled.c
@@ -272,7 +272,7 @@ static int gpio_keys_polled_probe(struct platform_device *pdev)
 
 	input = poll_dev->input;
 
-	input->name = pdev->name;
+	input->name = (pdata->name ? pdata->name : pdev->name);
 	input->phys = DRV_NAME"/input0";
 
 	input->id.bustype = BUS_HOST;
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH v1] HID: intel-ish-hid: Switch to use new generic UUID API
From: Srinivas Pandruvada @ 2019-01-22 17:29 UTC (permalink / raw)
  To: Andy Shevchenko, linux-input, Even Xu, linux-kernel,
	Christoph Hellwig
In-Reply-To: <20190110154804.89298-1-andriy.shevchenko@linux.intel.com>

On Thu, 2019-01-10 at 17:48 +0200, Andy Shevchenko wrote:
> There are new types and helpers that are supposed to be used in new
> code.
> 
> As a preparation to get rid of legacy types and API functions do
> the conversion here.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

> ---
>  drivers/hid/intel-ish-hid/ishtp-hid-client.c |  3 +--
>  drivers/hid/intel-ish-hid/ishtp-hid.h        |  6 +++---
>  drivers/hid/intel-ish-hid/ishtp/bus.c        | 19 ++++++++--------
> ---
>  drivers/hid/intel-ish-hid/ishtp/bus.h        |  4 ++--
>  drivers/hid/intel-ish-hid/ishtp/client.h     |  2 +-
>  drivers/hid/intel-ish-hid/ishtp/hbm.h        |  2 +-
>  6 files changed, 16 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> index e64243bc9c96..2246697ada1d 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c
> @@ -788,8 +788,7 @@ static int hid_ishtp_cl_probe(struct
> ishtp_cl_device *cl_device)
>  	if (!cl_device)
>  		return	-ENODEV;
>  
> -	if (uuid_le_cmp(hid_ishtp_guid,
> -			cl_device->fw_client->props.protocol_name) !=
> 0)
> +	if (!guid_equal(&hid_ishtp_guid, &cl_device->fw_client-
> >props.protocol_name))
>  		return	-ENODEV;
>  
>  	client_data = devm_kzalloc(&cl_device->dev,
> sizeof(*client_data),
> diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.h
> b/drivers/hid/intel-ish-hid/ishtp-hid.h
> index f5c7eb79b7b5..1cd07a441cd4 100644
> --- a/drivers/hid/intel-ish-hid/ishtp-hid.h
> +++ b/drivers/hid/intel-ish-hid/ishtp-hid.h
> @@ -29,9 +29,9 @@
>  		client->cl_device->ishtp_dev, __VA_ARGS__)
>  
>  /* ISH Transport protocol (ISHTP in short) GUID */
> -static const uuid_le hid_ishtp_guid = UUID_LE(0x33AECD58, 0xB679,
> 0x4E54,
> -					      0x9B, 0xD9, 0xA0, 0x4D,
> 0x34,
> -					      0xF0, 0xC2, 0x26);
> +static const guid_t hid_ishtp_guid =
> +	GUID_INIT(0x33AECD58, 0xB679, 0x4E54,
> +		  0x9B, 0xD9, 0xA0, 0x4D, 0x34, 0xF0, 0xC2, 0x26);
>  
>  /* ISH HID message structure */
>  struct hostif_msg_hdr {
> diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c
> b/drivers/hid/intel-ish-hid/ishtp/bus.c
> index 728dc6d4561a..1b6092b300fe 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/bus.c
> +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
> @@ -133,18 +133,15 @@ int ishtp_write_message(struct ishtp_device
> *dev, struct ishtp_msg_hdr *hdr,
>   *
>   * Return: fw client index or -ENOENT if not found
>   */
> -int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const uuid_le
> *uuid)
> +int ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const guid_t
> *uuid)
>  {
> -	int i, res = -ENOENT;
> +	unsigned int i;
>  
>  	for (i = 0; i < dev->fw_clients_num; ++i) {
> -		if (uuid_le_cmp(*uuid, dev-
> >fw_clients[i].props.protocol_name)
> -				== 0) {
> -			res = i;
> -			break;
> -		}
> +		if (guid_equal(uuid, &dev-
> >fw_clients[i].props.protocol_name))
> +			return i;
>  	}
> -	return res;
> +	return -ENOENT;
>  }
>  EXPORT_SYMBOL(ishtp_fw_cl_by_uuid);
>  
> @@ -158,7 +155,7 @@ EXPORT_SYMBOL(ishtp_fw_cl_by_uuid);
>   * Return: pointer of client information on success, NULL on
> failure.
>   */
>  struct ishtp_fw_client *ishtp_fw_cl_get_client(struct ishtp_device
> *dev,
> -						const uuid_le *uuid)
> +					       const guid_t *uuid)
>  {
>  	int i;
>  	unsigned long flags;
> @@ -401,7 +398,7 @@ static const struct device_type
> ishtp_cl_device_type = {
>   * Return: ishtp_cl_device pointer or NULL on failure
>   */
>  static struct ishtp_cl_device *ishtp_bus_add_device(struct
> ishtp_device *dev,
> -						    uuid_le uuid, char
> *name)
> +						    guid_t uuid, char
> *name)
>  {
>  	struct ishtp_cl_device *device;
>  	int status;
> @@ -629,7 +626,7 @@ int ishtp_bus_new_client(struct ishtp_device
> *dev)
>  	int	i;
>  	char	*dev_name;
>  	struct ishtp_cl_device	*cl_device;
> -	uuid_le	device_uuid;
> +	guid_t	device_uuid;
>  
>  	/*
>  	 * For all reported clients, create an unconnected client and
> add its
> diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.h
> b/drivers/hid/intel-ish-hid/ishtp/bus.h
> index b8a5bcc82536..babf19ba3ff6 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/bus.h
> +++ b/drivers/hid/intel-ish-hid/ishtp/bus.h
> @@ -112,8 +112,8 @@ void	ishtp_cl_driver_unregister(struct
> ishtp_cl_driver *driver);
>  
>  int	ishtp_register_event_cb(struct ishtp_cl_device *device,
>  				void (*read_cb)(struct ishtp_cl_device
> *));
> -int	ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const uuid_le
> *cuuid);
> +int	ishtp_fw_cl_by_uuid(struct ishtp_device *dev, const guid_t
> *cuuid);
>  struct	ishtp_fw_client *ishtp_fw_cl_get_client(struct
> ishtp_device *dev,
> -						const uuid_le *uuid);
> +						const guid_t *uuid);
>  
>  #endif /* _LINUX_ISHTP_CL_BUS_H */
> diff --git a/drivers/hid/intel-ish-hid/ishtp/client.h
> b/drivers/hid/intel-ish-hid/ishtp/client.h
> index 042f4c4853b1..e0df3eb611e6 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/client.h
> +++ b/drivers/hid/intel-ish-hid/ishtp/client.h
> @@ -126,7 +126,7 @@ struct ishtp_cl {
>  };
>  
>  /* Client connection managenment internal functions */
> -int ishtp_can_client_connect(struct ishtp_device *ishtp_dev, uuid_le
> *uuid);
> +int ishtp_can_client_connect(struct ishtp_device *ishtp_dev, guid_t
> *uuid);
>  int ishtp_fw_cl_by_id(struct ishtp_device *dev, uint8_t client_id);
>  void ishtp_cl_send_msg(struct ishtp_device *dev, struct ishtp_cl
> *cl);
>  void recv_ishtp_cl_msg(struct ishtp_device *dev,
> diff --git a/drivers/hid/intel-ish-hid/ishtp/hbm.h
> b/drivers/hid/intel-ish-hid/ishtp/hbm.h
> index d96111cef7f8..7286e3600140 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/hbm.h
> +++ b/drivers/hid/intel-ish-hid/ishtp/hbm.h
> @@ -149,7 +149,7 @@ struct hbm_host_enum_response {
>  } __packed;
>  
>  struct ishtp_client_properties {
> -	uuid_le protocol_name;
> +	guid_t protocol_name;
>  	uint8_t protocol_version;
>  	uint8_t max_number_of_connections;
>  	uint8_t fixed_address;

^ permalink raw reply

* [PATCH 1/2] dt-bindings: input: sitronix-st1232: add compatible string for ST1633
From: Martin Kepplinger @ 2019-01-23  7:26 UTC (permalink / raw)
  To: devicetree, linux-input
  Cc: dmitry.torokhov, robh+dt, mark.rutland, chinyeow.sim.xt,
	linux-kernel, Martin Kepplinger

From: Martin Kepplinger <martin.kepplinger@ginzinger.com>

The st1232 driver gains support for the ST1633 controller too; update
the bindings doc accordingly.

Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---
 .../bindings/input/touchscreen/sitronix-st1232.txt          | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt b/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
index 64ad48b824a2..e73e826e0f2a 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix-st1232.txt
@@ -1,7 +1,9 @@
-* Sitronix st1232 touchscreen controller
+* Sitronix st1232 or st1633 touchscreen controller
 
 Required properties:
-- compatible: must be "sitronix,st1232"
+- compatible: must contain one of
+  * "sitronix,st1232"
+  * "sitronix,st1633"
 - reg: I2C address of the chip
 - interrupts: interrupt to which the chip is connected
 
-- 
2.20.1

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox