Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCH v7 0/7] Introduce STPMIC1 PMIC Driver
From: Lee Jones @ 2018-12-05  9:36 UTC (permalink / raw)
  To: Pascal PAILLET-LME
  Cc: dmitry.torokhov@gmail.com, robh+dt@kernel.org,
	mark.rutland@arm.com, lgirdwood@gmail.com, broonie@kernel.org,
	wim@linux-watchdog.org, linux@roeck-us.net,
	linux-input@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org,
	benjamin.gaignard@linaro.org, eballetbo@gmail.com,
	axel.lin@ingics.com
In-Reply-To: <5C069C47.6080305@st.com>

On Tue, 04 Dec 2018, Pascal PAILLET-LME wrote:
> Le 12/03/2018 08:11 AM, Lee Jones a écrit :
> > On Fri, 30 Nov 2018, Pascal PAILLET-LME wrote:
> >
> >> The goal of this patch-set is to propose a driver for the STPMIC1 PMIC from
> >> STMicroelectronics.
> >> The STPMIC1 regulators supply power to an application processor as well as
> >> to external system peripherals such as DDR, Flash memories and system
> >> devices. It also features onkey button input and an hardware watchdog.
> >> The STPMIC1 is controlled via I2C.
> >>
> >> Main driver is drivers/mfd/stpmic1 that handle I2C regmap configuration and
> >> irqchip. stpmic1_regulator, stpmic1_onkey and stpmic1_wdt need stpmic1 mfd
> >> as parent.
> >>
> >> STPMIC1 MFD and regulator drivers maybe mandatory at boot time.
> >>
> >> Pascal Paillet (7):
> >> changes in v7:
> >> * rebase on regul/for-next
> >>
> >>    dt-bindings: mfd: document stpmic1
> >>    mfd: stpmic1: add stpmic1 driver
> >>    dt-bindings: input: document stpmic1 pmic onkey
> >>    input: stpmic1: add stpmic1 onkey driver
> >>    dt-bindings: watchdog: document stpmic1 pmic watchdog
> >>    watchdog: stpmic1: add stpmic1 watchdog driver
> >>    regulator: stpmic1: fix regulator_lock usage
> >>
> >>   .../devicetree/bindings/input/st,stpmic1-onkey.txt |  28 +++
> >>   .../devicetree/bindings/mfd/st,stpmic1.txt         |  61 ++++++
> >>   .../bindings/watchdog/st,stpmic1-wdt.txt           |  11 ++
> >>   drivers/input/misc/Kconfig                         |  11 ++
> >>   drivers/input/misc/Makefile                        |   2 +
> >>   drivers/input/misc/stpmic1_onkey.c                 | 198 +++++++++++++++++++
> >>   drivers/mfd/Kconfig                                |  16 ++
> >>   drivers/mfd/Makefile                               |   1 +
> >>   drivers/mfd/stpmic1.c                              | 213 +++++++++++++++++++++
> >>   drivers/regulator/stpmic1_regulator.c              |   2 +-
> > Is it just Mark you're waiting on now?
> It is ok now, Mark has applied the patch.
> Evrey thing has been acked now. What is the nex step ?

Mark has applied patch 7?

Why don't I see that?  Should you have removed it from this set?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* [PATCH AUTOSEL 4.19 038/123] iio/hid-sensors: Fix IIO_CHAN_INFO_RAW returning wrong values for signed numbers
From: Sasha Levin @ 2018-12-05  9:34 UTC (permalink / raw)
  To: stable, linux-kernel
  Cc: Hans de Goede, Stable, Jonathan Cameron, Sasha Levin, linux-input,
	linux-iio, linux-rtc
In-Reply-To: <20181205093555.5386-1-sashal@kernel.org>

From: Hans de Goede <hdegoede@redhat.com>

[ Upstream commit 0145b50566e7de5637e80ecba96c7f0e6fff1aad ]

Before this commit sensor_hub_input_attr_get_raw_value() failed to take
the signedness of 16 and 8 bit values into account, returning e.g.
65436 instead of -100 for the z-axis reading of an accelerometer.

This commit adds a new is_signed parameter to the function and makes all
callers pass the appropriate value for this.

While at it, this commit also fixes up some neighboring lines where
statements were needlessly split over 2 lines to improve readability.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: <Stable@vger.kernel.org>
Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/hid/hid-sensor-custom.c                  |  2 +-
 drivers/hid/hid-sensor-hub.c                     | 13 ++++++++++---
 drivers/iio/accel/hid-sensor-accel-3d.c          |  5 ++++-
 drivers/iio/gyro/hid-sensor-gyro-3d.c            |  5 ++++-
 drivers/iio/humidity/hid-sensor-humidity.c       |  3 ++-
 drivers/iio/light/hid-sensor-als.c               |  8 +++++---
 drivers/iio/light/hid-sensor-prox.c              |  8 +++++---
 drivers/iio/magnetometer/hid-sensor-magn-3d.c    |  8 +++++---
 drivers/iio/orientation/hid-sensor-incl-3d.c     |  8 +++++---
 drivers/iio/pressure/hid-sensor-press.c          |  8 +++++---
 drivers/iio/temperature/hid-sensor-temperature.c |  3 ++-
 drivers/rtc/rtc-hid-sensor-time.c                |  2 +-
 include/linux/hid-sensor-hub.h                   |  4 +++-
 13 files changed, 52 insertions(+), 25 deletions(-)

diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
index e8a114157f87..bb012bc032e0 100644
--- a/drivers/hid/hid-sensor-custom.c
+++ b/drivers/hid/hid-sensor-custom.c
@@ -358,7 +358,7 @@ static ssize_t show_value(struct device *dev, struct device_attribute *attr,
 						sensor_inst->hsdev,
 						sensor_inst->hsdev->usage,
 						usage, report_id,
-						SENSOR_HUB_SYNC);
+						SENSOR_HUB_SYNC, false);
 	} else if (!strncmp(name, "units", strlen("units")))
 		value = sensor_inst->fields[field_index].attribute.units;
 	else if (!strncmp(name, "unit-expo", strlen("unit-expo")))
diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 2b63487057c2..4256fdc5cd6d 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -299,7 +299,8 @@ EXPORT_SYMBOL_GPL(sensor_hub_get_feature);
 int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
 					u32 usage_id,
 					u32 attr_usage_id, u32 report_id,
-					enum sensor_hub_read_flags flag)
+					enum sensor_hub_read_flags flag,
+					bool is_signed)
 {
 	struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
 	unsigned long flags;
@@ -331,10 +332,16 @@ int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
 						&hsdev->pending.ready, HZ*5);
 		switch (hsdev->pending.raw_size) {
 		case 1:
-			ret_val = *(u8 *)hsdev->pending.raw_data;
+			if (is_signed)
+				ret_val = *(s8 *)hsdev->pending.raw_data;
+			else
+				ret_val = *(u8 *)hsdev->pending.raw_data;
 			break;
 		case 2:
-			ret_val = *(u16 *)hsdev->pending.raw_data;
+			if (is_signed)
+				ret_val = *(s16 *)hsdev->pending.raw_data;
+			else
+				ret_val = *(u16 *)hsdev->pending.raw_data;
 			break;
 		case 4:
 			ret_val = *(u32 *)hsdev->pending.raw_data;
diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index 41d97faf5013..38ff374a3ca4 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -149,6 +149,7 @@ static int accel_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 	struct hid_sensor_hub_device *hsdev =
 					accel_state->common_attributes.hsdev;
 
@@ -158,12 +159,14 @@ static int accel_3d_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&accel_state->common_attributes, true);
 		report_id = accel_state->accel[chan->scan_index].report_id;
+		min = accel_state->accel[chan->scan_index].logical_minimum;
 		address = accel_3d_addresses[chan->scan_index];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 					accel_state->common_attributes.hsdev,
 					hsdev->usage, address, report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
 		else {
 			*val = 0;
 			hid_sensor_power_state(&accel_state->common_attributes,
diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
index 36941e69f959..88e857c4baf4 100644
--- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
+++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
@@ -111,6 +111,7 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;
@@ -118,13 +119,15 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&gyro_state->common_attributes, true);
 		report_id = gyro_state->gyro[chan->scan_index].report_id;
+		min = gyro_state->gyro[chan->scan_index].logical_minimum;
 		address = gyro_3d_addresses[chan->scan_index];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 					gyro_state->common_attributes.hsdev,
 					HID_USAGE_SENSOR_GYRO_3D, address,
 					report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
 		else {
 			*val = 0;
 			hid_sensor_power_state(&gyro_state->common_attributes,
diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
index beab6d6fd6e1..4bc95f31c730 100644
--- a/drivers/iio/humidity/hid-sensor-humidity.c
+++ b/drivers/iio/humidity/hid-sensor-humidity.c
@@ -75,7 +75,8 @@ static int humidity_read_raw(struct iio_dev *indio_dev,
 				HID_USAGE_SENSOR_HUMIDITY,
 				HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY,
 				humid_st->humidity_attr.report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				humid_st->humidity_attr.logical_minimum < 0);
 		hid_sensor_power_state(&humid_st->common_attributes, false);
 
 		return IIO_VAL_INT;
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 406caaee9a3c..94f33250ba5a 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -93,6 +93,7 @@ static int als_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;
@@ -102,8 +103,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
 		case  CHANNEL_SCAN_INDEX_INTENSITY:
 		case  CHANNEL_SCAN_INDEX_ILLUM:
 			report_id = als_state->als_illum.report_id;
-			address =
-			HID_USAGE_SENSOR_LIGHT_ILLUM;
+			min = als_state->als_illum.logical_minimum;
+			address = HID_USAGE_SENSOR_LIGHT_ILLUM;
 			break;
 		default:
 			report_id = -1;
@@ -116,7 +117,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
 					als_state->common_attributes.hsdev,
 					HID_USAGE_SENSOR_ALS, address,
 					report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
 			hid_sensor_power_state(&als_state->common_attributes,
 						false);
 		} else {
diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
index 45107f7537b5..cf5a0c242609 100644
--- a/drivers/iio/light/hid-sensor-prox.c
+++ b/drivers/iio/light/hid-sensor-prox.c
@@ -73,6 +73,7 @@ static int prox_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;
@@ -81,8 +82,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
 		switch (chan->scan_index) {
 		case  CHANNEL_SCAN_INDEX_PRESENCE:
 			report_id = prox_state->prox_attr.report_id;
-			address =
-			HID_USAGE_SENSOR_HUMAN_PRESENCE;
+			min = prox_state->prox_attr.logical_minimum;
+			address = HID_USAGE_SENSOR_HUMAN_PRESENCE;
 			break;
 		default:
 			report_id = -1;
@@ -95,7 +96,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
 				prox_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_PROX, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 			hid_sensor_power_state(&prox_state->common_attributes,
 						false);
 		} else {
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index d55c4885211a..f3c0d41e5a8c 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -163,21 +163,23 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&magn_state->magn_flux_attributes, true);
-		report_id =
-			magn_state->magn[chan->address].report_id;
+		report_id = magn_state->magn[chan->address].report_id;
+		min = magn_state->magn[chan->address].logical_minimum;
 		address = magn_3d_addresses[chan->address];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 				magn_state->magn_flux_attributes.hsdev,
 				HID_USAGE_SENSOR_COMPASS_3D, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 		else {
 			*val = 0;
 			hid_sensor_power_state(
diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c
index 1e5451d1ff88..bdc5e4554ee4 100644
--- a/drivers/iio/orientation/hid-sensor-incl-3d.c
+++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
@@ -111,21 +111,23 @@ static int incl_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&incl_state->common_attributes, true);
-		report_id =
-			incl_state->incl[chan->scan_index].report_id;
+		report_id = incl_state->incl[chan->scan_index].report_id;
+		min = incl_state->incl[chan->scan_index].logical_minimum;
 		address = incl_3d_addresses[chan->scan_index];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 				incl_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_INCLINOMETER_3D, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 		else {
 			hid_sensor_power_state(&incl_state->common_attributes,
 						false);
diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
index 4c437918f1d2..d7b1c00ceb4d 100644
--- a/drivers/iio/pressure/hid-sensor-press.c
+++ b/drivers/iio/pressure/hid-sensor-press.c
@@ -77,6 +77,7 @@ static int press_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;
@@ -85,8 +86,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
 		switch (chan->scan_index) {
 		case  CHANNEL_SCAN_INDEX_PRESSURE:
 			report_id = press_state->press_attr.report_id;
-			address =
-			HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
+			min = press_state->press_attr.logical_minimum;
+			address = HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
 			break;
 		default:
 			report_id = -1;
@@ -99,7 +100,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
 				press_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_PRESSURE, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 			hid_sensor_power_state(&press_state->common_attributes,
 						false);
 		} else {
diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
index beaf6fd3e337..b592fc4f007e 100644
--- a/drivers/iio/temperature/hid-sensor-temperature.c
+++ b/drivers/iio/temperature/hid-sensor-temperature.c
@@ -76,7 +76,8 @@ static int temperature_read_raw(struct iio_dev *indio_dev,
 			HID_USAGE_SENSOR_TEMPERATURE,
 			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
 			temp_st->temperature_attr.report_id,
-			SENSOR_HUB_SYNC);
+			SENSOR_HUB_SYNC,
+			temp_st->temperature_attr.logical_minimum < 0);
 		hid_sensor_power_state(
 				&temp_st->common_attributes,
 				false);
diff --git a/drivers/rtc/rtc-hid-sensor-time.c b/drivers/rtc/rtc-hid-sensor-time.c
index 2751dba850c6..3e1abb455472 100644
--- a/drivers/rtc/rtc-hid-sensor-time.c
+++ b/drivers/rtc/rtc-hid-sensor-time.c
@@ -213,7 +213,7 @@ static int hid_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	/* get a report with all values through requesting one value */
 	sensor_hub_input_attr_get_raw_value(time_state->common_attributes.hsdev,
 			HID_USAGE_SENSOR_TIME, hid_time_addresses[0],
-			time_state->info[0].report_id, SENSOR_HUB_SYNC);
+			time_state->info[0].report_id, SENSOR_HUB_SYNC, false);
 	/* wait for all values (event) */
 	ret = wait_for_completion_killable_timeout(
 			&time_state->comp_last_time, HZ*6);
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index 331dc377c275..dc12f5c4b076 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -177,6 +177,7 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 * @attr_usage_id:	Attribute usage id as per spec
 * @report_id:	Report id to look for
 * @flag:      Synchronous or asynchronous read
+* @is_signed:   If true then fields < 32 bits will be sign-extended
 *
 * Issues a synchronous or asynchronous read request for an input attribute.
 * Returns data upto 32 bits.
@@ -190,7 +191,8 @@ enum sensor_hub_read_flags {
 int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
  					u32 usage_id,
  					u32 attr_usage_id, u32 report_id,
- 					enum sensor_hub_read_flags flag
+					enum sensor_hub_read_flags flag,
+					bool is_signed
 );
 
 /**
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH] HID: input: support Microsoft wireless radio control hotkey
From: Chris Chiu @ 2018-12-05  0:56 UTC (permalink / raw)
  To: Benjamin Tissoires
  Cc: Jiri Kosina, rydberg, linux-input, Linux Kernel,
	Linux Upstreaming Team
In-Reply-To: <CAO-hwJKAnHqPi7aOGqKET+_JKnXmvdwOyhtH4nLeqS+-K8597Q@mail.gmail.com>

On Fri, Nov 30, 2018 at 7:18 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Fri, Nov 30, 2018 at 7:46 AM Chris Chiu <chiu@endlessm.com> wrote:
> >
> > The ASUS laptops start to support the airplane mode radio management
> > to replace the original machanism of airplane mode toggle hotkey.
> > On the ASUS P5440FF, it presents as a HID device connecting via
> > I2C, name i2c-AMPD0001. When pressing hotkey, the Embedded Controller
> > send hid report up via I2C and switch the airplane mode indicator
> > LED based on the status.
> >
> > However, it's not working because it fails to be identified as a
> > hidinput device. It fails in hidinput_connect() due to the macro
> > IS_INPUT_APPLICATION doesn't identify HID_GD_WIRELESS_RADIO_CTLS
> > as a legit application code.
> >
> > It's easy to add the HID I2C vendor and product id to the quirk
> > list and apply HID_QUIRK_HIDINPUT_FORCE to make it work. But it
> > can be more generic to support such kind of application on PC.
>
> Sounds good, but while you are at it, can you please:
> - fix the kbuild warning
> - rewrite the whole line to use macros,
> - make the macro prettier by inserting new lines were required
> - and define the missing macro:
>   0x000d0006 -> HID_DG_WHITEBOARD
>
> Maybe we should keep the ranges definitions with raw values and put a
> comment on the side with the names.
>
> Cheers,
> Benjamin
>

Hi Bejamin,
    Thanks for your comment, I've submitted other patches which may
fulfill your suggestion.
https://lore.kernel.org/patchwork/patch/1020373/
https://lore.kernel.org/patchwork/patch/1020374/

     Please help me review and correct me if there's any modification
required. Thanks & Regards.

Chris

> >
> > Signed-off-by: Chris Chiu <chiu@endlessm.com>
> > ---
> >  include/linux/hid.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > index d44a78362942..f4805f605fed 100644
> > --- a/include/linux/hid.h
> > +++ b/include/linux/hid.h
> > @@ -836,7 +836,7 @@ static inline bool hid_is_using_ll_driver(struct hid_device *hdev,
> >
> >  /* Applications from HID Usage Tables 4/8/99 Version 1.1 */
> >  /* We ignore a few input applications that are not widely used */
> > -#define IS_INPUT_APPLICATION(a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006)))
> > +#define IS_INPUT_APPLICATION(a) (((a >= 0x00010000) && (a <= 0x00010008)) || (a == 0x00010080) || || (a == 0x0001000c) || (a == 0x000c0001) || ((a >= 0x000d0002) && (a <= 0x000d0006)))
> >
> >  /* HID core API */
> >
> > --
> > 2.11.0
> >

^ permalink raw reply

* [PATCH v3 8/8] HID: logitech: Use LDJ_DEVICE macro for existing Logitech mice
From: Peter Hutterer @ 2018-12-05  0:42 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Jiri Kosina, Harry Cutts, torvalds,
	Nestor Lopez Casado, linux-kernel, Benjamin Tissoires
In-Reply-To: <20181205004228.10714-1-peter.hutterer@who-t.net>

From: Harry Cutts <hcutts@chromium.org>

Signed-off-by: Harry Cutts <hcutts@chromium.org>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
Changes to v1, v2:
- reordered from 6/8 to 8/8 so the intermediate steps all build

 drivers/hid/hid-logitech-hidpp.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index a66daf8acd87..15ed6177a7a3 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -3362,13 +3362,11 @@ static void hidpp_remove(struct hid_device *hdev)
 
 static const struct hid_device_id hidpp_devices[] = {
 	{ /* wireless touchpad */
-	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
-		USB_VENDOR_ID_LOGITECH, 0x4011),
+	  LDJ_DEVICE(0x4011),
 	  .driver_data = HIDPP_QUIRK_CLASS_WTP | HIDPP_QUIRK_DELAYED_INIT |
 			 HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS },
 	{ /* wireless touchpad T650 */
-	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
-		USB_VENDOR_ID_LOGITECH, 0x4101),
+	  LDJ_DEVICE(0x4101),
 	  .driver_data = HIDPP_QUIRK_CLASS_WTP | HIDPP_QUIRK_DELAYED_INIT },
 	{ /* wireless touchpad T651 */
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH,
@@ -3408,16 +3406,13 @@ static const struct hid_device_id hidpp_devices[] = {
 	{ /* Mouse Logitech Performance MX */
 	  LDJ_DEVICE(0x101a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
 	{ /* Keyboard logitech K400 */
-	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
-		USB_VENDOR_ID_LOGITECH, 0x4024),
+	  LDJ_DEVICE(0x4024),
 	  .driver_data = HIDPP_QUIRK_CLASS_K400 },
 	{ /* Solar Keyboard Logitech K750 */
-	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
-		USB_VENDOR_ID_LOGITECH, 0x4002),
+	  LDJ_DEVICE(0x4002),
 	  .driver_data = HIDPP_QUIRK_CLASS_K750 },
 
-	{ HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
-		USB_VENDOR_ID_LOGITECH, HID_ANY_ID)},
+	{ LDJ_DEVICE(HID_ANY_ID) },
 
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_G920_WHEEL),
 		.driver_data = HIDPP_QUIRK_CLASS_G920 | HIDPP_QUIRK_FORCE_OUTPUT_REPORTS},
-- 
2.19.2

^ permalink raw reply related

* [PATCH v3 7/8] HID: logitech: Enable high-resolution scrolling on Logitech mice
From: Peter Hutterer @ 2018-12-05  0:42 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Jiri Kosina, Harry Cutts, torvalds,
	Nestor Lopez Casado, linux-kernel, Benjamin Tissoires
In-Reply-To: <20181205004228.10714-1-peter.hutterer@who-t.net>

From: Harry Cutts <hcutts@chromium.org>

There are three features used by various Logitech mice for
high-resolution scrolling: the scrolling acceleration bit in HID++ 1.0,
and the x2120 and x2121 features in HID++ 2.0 and above. This patch
supports all three, and uses the multiplier reported by the mouse for
the HID++ 2.0+ features.

The full list of product IDs of mice which support high-resolution
scrolling was provided by Logitech, but the patch was tested using the
following mice (using the Unifying receiver):

* HID++ 1.0: Anywhere MX, Performance MX
* x2120: M560
* x2121: MX Anywhere 2, MX Master 2S

This patch is a combinations of the now-reverted commits 1ff2e1a44e0,
d56ca9855bf9, 5fe2ccbef9d, 044ee89028 together with some extra bits for the
directional and timeout-based reset.
The previous patch series was in hid-input, it appears this remainder
handling is logitech-specific and was moved to hid-logitech-hidpp.c and
renamed accordingly.

Signed-off-by: Harry Cutts <hcutts@chromium.org>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
Changes to v1:
- get rid of the timer in favour of sched_clock()
- drop storing the multiplier and calculate value on the fly

Changes to v2:
- m560 now has REL_HWHEEL_HI_RES (untested, I don't have the device)


 drivers/hid/hid-logitech-hidpp.c | 301 ++++++++++++++++++++++++++++++-
 1 file changed, 295 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 95ba9db6e613..a66daf8acd87 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -21,6 +21,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
+#include <linux/sched/clock.h>
 #include <linux/kfifo.h>
 #include <linux/input/mt.h>
 #include <linux/workqueue.h>
@@ -64,6 +65,14 @@ MODULE_PARM_DESC(disable_tap_to_click,
 #define HIDPP_QUIRK_NO_HIDINPUT			BIT(23)
 #define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS	BIT(24)
 #define HIDPP_QUIRK_UNIFYING			BIT(25)
+#define HIDPP_QUIRK_HI_RES_SCROLL_1P0		BIT(26)
+#define HIDPP_QUIRK_HI_RES_SCROLL_X2120		BIT(27)
+#define HIDPP_QUIRK_HI_RES_SCROLL_X2121		BIT(28)
+
+/* Convenience constant to check for any high-res support. */
+#define HIDPP_QUIRK_HI_RES_SCROLL	(HIDPP_QUIRK_HI_RES_SCROLL_1P0 | \
+					 HIDPP_QUIRK_HI_RES_SCROLL_X2120 | \
+					 HIDPP_QUIRK_HI_RES_SCROLL_X2121)
 
 #define HIDPP_QUIRK_DELAYED_INIT		HIDPP_QUIRK_NO_HIDINPUT
 
@@ -128,6 +137,25 @@ struct hidpp_battery {
 	bool online;
 };
 
+/**
+ * struct hidpp_scroll_counter - Utility class for processing high-resolution
+ *                             scroll events.
+ * @dev: the input device for which events should be reported.
+ * @wheel_multiplier: the scalar multiplier to be applied to each wheel event
+ * @remainder: counts the number of high-resolution units moved since the last
+ *             low-resolution event (REL_WHEEL or REL_HWHEEL) was sent. Should
+ *             only be used by class methods.
+ * @direction: direction of last movement (1 or -1)
+ * @last_time: last event time, used to reset remainder after inactivity
+ */
+struct hidpp_scroll_counter {
+	struct input_dev *dev;
+	int wheel_multiplier;
+	int remainder;
+	int direction;
+	unsigned long long last_time;
+};
+
 struct hidpp_device {
 	struct hid_device *hid_dev;
 	struct mutex send_mutex;
@@ -149,6 +177,7 @@ struct hidpp_device {
 	unsigned long capabilities;
 
 	struct hidpp_battery battery;
+	struct hidpp_scroll_counter vertical_wheel_counter;
 };
 
 /* HID++ 1.0 error codes */
@@ -391,6 +420,67 @@ static void hidpp_prefix_name(char **name, int name_length)
 	*name = new_name;
 }
 
+/**
+ * hidpp_scroll_counter_handle_scroll() - Send high- and low-resolution scroll
+ *                                        events given a high-resolution wheel
+ *                                        movement.
+ * @counter: a hid_scroll_counter struct describing the wheel.
+ * @hi_res_value: the movement of the wheel, in the mouse's high-resolution
+ *                units.
+ *
+ * Given a high-resolution movement, this function converts the movement into
+ * fractions of 120 and emits high-resolution scroll events for the input
+ * device. It also uses the multiplier from &struct hid_scroll_counter to
+ * emit low-resolution scroll events when appropriate for
+ * backwards-compatibility with userspace input libraries.
+ */
+static void hidpp_scroll_counter_handle_scroll(struct hidpp_scroll_counter *counter,
+					       int hi_res_value)
+{
+	int low_res_value, remainder, direction;
+	unsigned long long now, previous;
+
+	hi_res_value = hi_res_value * 120/counter->wheel_multiplier;
+	input_report_rel(counter->dev, REL_WHEEL_HI_RES, hi_res_value);
+
+	remainder = counter->remainder;
+	direction = hi_res_value > 0 ? 1 : -1;
+
+	now = sched_clock();
+	previous = counter->last_time;
+	counter->last_time = now;
+	/*
+	 * Reset the remainder after a period of inactivity or when the
+	 * direction changes. This prevents the REL_WHEEL emulation point
+	 * from sliding for devices that don't always provide the same
+	 * number of movements per detent.
+	 */
+	if (now - previous > 1000000000 || direction != counter->direction)
+		remainder = 0;
+
+	counter->direction = direction;
+	remainder += hi_res_value;
+
+	/* Some wheels will rest 7/8ths of a detent from the previous detent
+	 * after slow movement, so we want the threshold for low-res events to
+	 * be in the middle between two detents (e.g. after 4/8ths) as
+	 * opposed to on the detents themselves (8/8ths).
+	 */
+	if (abs(remainder) >= 60) {
+		/* Add (or subtract) 1 because we want to trigger when the wheel
+		 * is half-way to the next detent (i.e. scroll 1 detent after a
+		 * 1/2 detent movement, 2 detents after a 1 1/2 detent movement,
+		 * etc.).
+		 */
+		low_res_value = remainder / 120;
+		if (low_res_value == 0)
+			low_res_value = (hi_res_value > 0 ? 1 : -1);
+		input_report_rel(counter->dev, REL_WHEEL, low_res_value);
+		remainder -= low_res_value * 120;
+	}
+	counter->remainder = remainder;
+}
+
 /* -------------------------------------------------------------------------- */
 /* HIDP++ 1.0 commands                                                        */
 /* -------------------------------------------------------------------------- */
@@ -1157,6 +1247,99 @@ static int hidpp_battery_get_property(struct power_supply *psy,
 	return ret;
 }
 
+/* -------------------------------------------------------------------------- */
+/* 0x2120: Hi-resolution scrolling                                            */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_PAGE_HI_RESOLUTION_SCROLLING			0x2120
+
+#define CMD_HI_RESOLUTION_SCROLLING_SET_HIGHRES_SCROLLING_MODE	0x10
+
+static int hidpp_hrs_set_highres_scrolling_mode(struct hidpp_device *hidpp,
+	bool enabled, u8 *multiplier)
+{
+	u8 feature_index;
+	u8 feature_type;
+	int ret;
+	u8 params[1];
+	struct hidpp_report response;
+
+	ret = hidpp_root_get_feature(hidpp,
+				     HIDPP_PAGE_HI_RESOLUTION_SCROLLING,
+				     &feature_index,
+				     &feature_type);
+	if (ret)
+		return ret;
+
+	params[0] = enabled ? BIT(0) : 0;
+	ret = hidpp_send_fap_command_sync(hidpp, feature_index,
+					  CMD_HI_RESOLUTION_SCROLLING_SET_HIGHRES_SCROLLING_MODE,
+					  params, sizeof(params), &response);
+	if (ret)
+		return ret;
+	*multiplier = response.fap.params[1];
+	return 0;
+}
+
+/* -------------------------------------------------------------------------- */
+/* 0x2121: HiRes Wheel                                                        */
+/* -------------------------------------------------------------------------- */
+
+#define HIDPP_PAGE_HIRES_WHEEL		0x2121
+
+#define CMD_HIRES_WHEEL_GET_WHEEL_CAPABILITY	0x00
+#define CMD_HIRES_WHEEL_SET_WHEEL_MODE		0x20
+
+static int hidpp_hrw_get_wheel_capability(struct hidpp_device *hidpp,
+	u8 *multiplier)
+{
+	u8 feature_index;
+	u8 feature_type;
+	int ret;
+	struct hidpp_report response;
+
+	ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HIRES_WHEEL,
+				     &feature_index, &feature_type);
+	if (ret)
+		goto return_default;
+
+	ret = hidpp_send_fap_command_sync(hidpp, feature_index,
+					  CMD_HIRES_WHEEL_GET_WHEEL_CAPABILITY,
+					  NULL, 0, &response);
+	if (ret)
+		goto return_default;
+
+	*multiplier = response.fap.params[0];
+	return 0;
+return_default:
+	hid_warn(hidpp->hid_dev,
+		 "Couldn't get wheel multiplier (error %d)\n", ret);
+	return ret;
+}
+
+static int hidpp_hrw_set_wheel_mode(struct hidpp_device *hidpp, bool invert,
+	bool high_resolution, bool use_hidpp)
+{
+	u8 feature_index;
+	u8 feature_type;
+	int ret;
+	u8 params[1];
+	struct hidpp_report response;
+
+	ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HIRES_WHEEL,
+				     &feature_index, &feature_type);
+	if (ret)
+		return ret;
+
+	params[0] = (invert          ? BIT(2) : 0) |
+		    (high_resolution ? BIT(1) : 0) |
+		    (use_hidpp       ? BIT(0) : 0);
+
+	return hidpp_send_fap_command_sync(hidpp, feature_index,
+					   CMD_HIRES_WHEEL_SET_WHEEL_MODE,
+					   params, sizeof(params), &response);
+}
+
 /* -------------------------------------------------------------------------- */
 /* 0x4301: Solar Keyboard                                                     */
 /* -------------------------------------------------------------------------- */
@@ -2408,10 +2591,15 @@ static int m560_raw_event(struct hid_device *hdev, u8 *data, int size)
 		input_report_key(mydata->input, BTN_RIGHT,
 			!!(data[1] & M560_MOUSE_BTN_RIGHT));
 
-		if (data[1] & M560_MOUSE_BTN_WHEEL_LEFT)
+		if (data[1] & M560_MOUSE_BTN_WHEEL_LEFT) {
 			input_report_rel(mydata->input, REL_HWHEEL, -1);
-		else if (data[1] & M560_MOUSE_BTN_WHEEL_RIGHT)
+			input_report_rel(mydata->input, REL_HWHEEL_HI_RES,
+					 -120);
+		} else if (data[1] & M560_MOUSE_BTN_WHEEL_RIGHT) {
 			input_report_rel(mydata->input, REL_HWHEEL, 1);
+			input_report_rel(mydata->input, REL_HWHEEL_HI_RES,
+					 120);
+		}
 
 		v = hid_snto32(hid_field_extract(hdev, data+3, 0, 12), 12);
 		input_report_rel(mydata->input, REL_X, v);
@@ -2420,7 +2608,8 @@ static int m560_raw_event(struct hid_device *hdev, u8 *data, int size)
 		input_report_rel(mydata->input, REL_Y, v);
 
 		v = hid_snto32(data[6], 8);
-		input_report_rel(mydata->input, REL_WHEEL, v);
+		hidpp_scroll_counter_handle_scroll(
+				&hidpp->vertical_wheel_counter, v);
 
 		input_sync(mydata->input);
 	}
@@ -2447,6 +2636,8 @@ static void m560_populate_input(struct hidpp_device *hidpp,
 	__set_bit(REL_Y, mydata->input->relbit);
 	__set_bit(REL_WHEEL, mydata->input->relbit);
 	__set_bit(REL_HWHEEL, mydata->input->relbit);
+	__set_bit(REL_WHEEL_HI_RES, mydata->input->relbit);
+	__set_bit(REL_HWHEEL_HI_RES, mydata->input->relbit);
 }
 
 static int m560_input_mapping(struct hid_device *hdev, struct hid_input *hi,
@@ -2548,6 +2739,37 @@ static int g920_get_config(struct hidpp_device *hidpp)
 	return 0;
 }
 
+/* -------------------------------------------------------------------------- */
+/* High-resolution scroll wheels                                              */
+/* -------------------------------------------------------------------------- */
+
+static int hi_res_scroll_enable(struct hidpp_device *hidpp)
+{
+	int ret;
+	u8 multiplier = 1;
+
+	if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2121) {
+		ret = hidpp_hrw_set_wheel_mode(hidpp, false, true, false);
+		if (ret == 0)
+			ret = hidpp_hrw_get_wheel_capability(hidpp, &multiplier);
+	} else if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_X2120) {
+		ret = hidpp_hrs_set_highres_scrolling_mode(hidpp, true,
+							   &multiplier);
+	} else /* if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL_1P0) */ {
+		ret = hidpp10_enable_scrolling_acceleration(hidpp);
+		multiplier = 8;
+	}
+	if (ret)
+		return ret;
+
+	if (multiplier == 0)
+		multiplier = 1;
+
+	hidpp->vertical_wheel_counter.wheel_multiplier = multiplier;
+	hid_info(hidpp->hid_dev, "multiplier = %d\n", multiplier);
+	return 0;
+}
+
 /* -------------------------------------------------------------------------- */
 /* Generic HID++ devices                                                      */
 /* -------------------------------------------------------------------------- */
@@ -2593,6 +2815,9 @@ static void hidpp_populate_input(struct hidpp_device *hidpp,
 		wtp_populate_input(hidpp, input, origin_is_hid_core);
 	else if (hidpp->quirks & HIDPP_QUIRK_CLASS_M560)
 		m560_populate_input(hidpp, input, origin_is_hid_core);
+
+	if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL)
+		hidpp->vertical_wheel_counter.dev = input;
 }
 
 static int hidpp_input_configured(struct hid_device *hdev,
@@ -2711,6 +2936,27 @@ static int hidpp_raw_event(struct hid_device *hdev, struct hid_report *report,
 	return 0;
 }
 
+static int hidpp_event(struct hid_device *hdev, struct hid_field *field,
+	struct hid_usage *usage, __s32 value)
+{
+	/* This function will only be called for scroll events, due to the
+	 * restriction imposed in hidpp_usages.
+	 */
+	struct hidpp_device *hidpp = hid_get_drvdata(hdev);
+	struct hidpp_scroll_counter *counter = &hidpp->vertical_wheel_counter;
+	/* A scroll event may occur before the multiplier has been retrieved or
+	 * the input device set, or high-res scroll enabling may fail. In such
+	 * cases we must return early (falling back to default behaviour) to
+	 * avoid a crash in hidpp_scroll_counter_handle_scroll.
+	 */
+	if (!(hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL) || value == 0
+	    || counter->dev == NULL || counter->wheel_multiplier == 0)
+		return 0;
+
+	hidpp_scroll_counter_handle_scroll(counter, value);
+	return 1;
+}
+
 static int hidpp_initialize_battery(struct hidpp_device *hidpp)
 {
 	static atomic_t battery_no = ATOMIC_INIT(0);
@@ -2922,6 +3168,9 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 	if (hidpp->battery.ps)
 		power_supply_changed(hidpp->battery.ps);
 
+	if (hidpp->quirks & HIDPP_QUIRK_HI_RES_SCROLL)
+		hi_res_scroll_enable(hidpp);
+
 	if (!(hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT) || hidpp->delayed_input)
 		/* if the input nodes are already created, we can stop now */
 		return;
@@ -3107,6 +3356,10 @@ static void hidpp_remove(struct hid_device *hdev)
 	mutex_destroy(&hidpp->send_mutex);
 }
 
+#define LDJ_DEVICE(product) \
+	HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE, \
+		   USB_VENDOR_ID_LOGITECH, (product))
+
 static const struct hid_device_id hidpp_devices[] = {
 	{ /* wireless touchpad */
 	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
@@ -3121,10 +3374,39 @@ static const struct hid_device_id hidpp_devices[] = {
 	  HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH,
 		USB_DEVICE_ID_LOGITECH_T651),
 	  .driver_data = HIDPP_QUIRK_CLASS_WTP },
+	{ /* Mouse Logitech Anywhere MX */
+	  LDJ_DEVICE(0x1017), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
+	{ /* Mouse Logitech Cube */
+	  LDJ_DEVICE(0x4010), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2120 },
+	{ /* Mouse Logitech M335 */
+	  LDJ_DEVICE(0x4050), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+	{ /* Mouse Logitech M515 */
+	  LDJ_DEVICE(0x4007), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2120 },
 	{ /* Mouse logitech M560 */
-	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
-		USB_VENDOR_ID_LOGITECH, 0x402d),
-	  .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560 },
+	  LDJ_DEVICE(0x402d),
+	  .driver_data = HIDPP_QUIRK_DELAYED_INIT | HIDPP_QUIRK_CLASS_M560
+		| HIDPP_QUIRK_HI_RES_SCROLL_X2120 },
+	{ /* Mouse Logitech M705 (firmware RQM17) */
+	  LDJ_DEVICE(0x101b), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
+	{ /* Mouse Logitech M705 (firmware RQM67) */
+	  LDJ_DEVICE(0x406d), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+	{ /* Mouse Logitech M720 */
+	  LDJ_DEVICE(0x405e), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+	{ /* Mouse Logitech MX Anywhere 2 */
+	  LDJ_DEVICE(0x404a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+	{ LDJ_DEVICE(0xb013), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+	{ LDJ_DEVICE(0xb018), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+	{ LDJ_DEVICE(0xb01f), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+	{ /* Mouse Logitech MX Anywhere 2S */
+	  LDJ_DEVICE(0x406a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+	{ /* Mouse Logitech MX Master */
+	  LDJ_DEVICE(0x4041), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+	{ LDJ_DEVICE(0x4060), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+	{ LDJ_DEVICE(0x4071), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+	{ /* Mouse Logitech MX Master 2S */
+	  LDJ_DEVICE(0x4069), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_X2121 },
+	{ /* Mouse Logitech Performance MX */
+	  LDJ_DEVICE(0x101a), .driver_data = HIDPP_QUIRK_HI_RES_SCROLL_1P0 },
 	{ /* Keyboard logitech K400 */
 	  HID_DEVICE(BUS_USB, HID_GROUP_LOGITECH_DJ_DEVICE,
 		USB_VENDOR_ID_LOGITECH, 0x4024),
@@ -3144,12 +3426,19 @@ static const struct hid_device_id hidpp_devices[] = {
 
 MODULE_DEVICE_TABLE(hid, hidpp_devices);
 
+static const struct hid_usage_id hidpp_usages[] = {
+	{ HID_GD_WHEEL, EV_REL, REL_WHEEL_HI_RES },
+	{ HID_ANY_ID - 1, HID_ANY_ID - 1, HID_ANY_ID - 1}
+};
+
 static struct hid_driver hidpp_driver = {
 	.name = "logitech-hidpp-device",
 	.id_table = hidpp_devices,
 	.probe = hidpp_probe,
 	.remove = hidpp_remove,
 	.raw_event = hidpp_raw_event,
+	.usage_table = hidpp_usages,
+	.event = hidpp_event,
 	.input_configured = hidpp_input_configured,
 	.input_mapping = hidpp_input_mapping,
 	.input_mapped = hidpp_input_mapped,
-- 
2.19.2

^ permalink raw reply related

* [PATCH v3 6/8] HID: logitech: Add function to enable HID++ 1.0 "scrolling acceleration"
From: Peter Hutterer @ 2018-12-05  0:42 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Jiri Kosina, Harry Cutts, torvalds,
	Nestor Lopez Casado, linux-kernel, Benjamin Tissoires
In-Reply-To: <20181205004228.10714-1-peter.hutterer@who-t.net>

From: Harry Cutts <hcutts@chromium.org>

"Scrolling acceleration" is a bit of a misnomer: it doesn't deal with
acceleration at all. However, that's the name used in Logitech's spec,
so I used it here.

Signed-off-by: Harry Cutts <hcutts@chromium.org>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
Unchanged since v1

 drivers/hid/hid-logitech-hidpp.c | 47 +++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 22b37a3844d1..95ba9db6e613 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -400,32 +400,53 @@ static void hidpp_prefix_name(char **name, int name_length)
 #define HIDPP_SET_LONG_REGISTER				0x82
 #define HIDPP_GET_LONG_REGISTER				0x83
 
-#define HIDPP_REG_GENERAL				0x00
-
-static int hidpp10_enable_battery_reporting(struct hidpp_device *hidpp_dev)
+/**
+ * hidpp10_set_register_bit() - Sets a single bit in a HID++ 1.0 register.
+ * @hidpp_dev: the device to set the register on.
+ * @register_address: the address of the register to modify.
+ * @byte: the byte of the register to modify. Should be less than 3.
+ * Return: 0 if successful, otherwise a negative error code.
+ */
+static int hidpp10_set_register_bit(struct hidpp_device *hidpp_dev,
+	u8 register_address, u8 byte, u8 bit)
 {
 	struct hidpp_report response;
 	int ret;
 	u8 params[3] = { 0 };
 
 	ret = hidpp_send_rap_command_sync(hidpp_dev,
-					REPORT_ID_HIDPP_SHORT,
-					HIDPP_GET_REGISTER,
-					HIDPP_REG_GENERAL,
-					NULL, 0, &response);
+					  REPORT_ID_HIDPP_SHORT,
+					  HIDPP_GET_REGISTER,
+					  register_address,
+					  NULL, 0, &response);
 	if (ret)
 		return ret;
 
 	memcpy(params, response.rap.params, 3);
 
-	/* Set the battery bit */
-	params[0] |= BIT(4);
+	params[byte] |= BIT(bit);
 
 	return hidpp_send_rap_command_sync(hidpp_dev,
-					REPORT_ID_HIDPP_SHORT,
-					HIDPP_SET_REGISTER,
-					HIDPP_REG_GENERAL,
-					params, 3, &response);
+					   REPORT_ID_HIDPP_SHORT,
+					   HIDPP_SET_REGISTER,
+					   register_address,
+					   params, 3, &response);
+}
+
+
+#define HIDPP_REG_GENERAL				0x00
+
+static int hidpp10_enable_battery_reporting(struct hidpp_device *hidpp_dev)
+{
+	return hidpp10_set_register_bit(hidpp_dev, HIDPP_REG_GENERAL, 0, 4);
+}
+
+#define HIDPP_REG_FEATURES				0x01
+
+/* On HID++ 1.0 devices, high-res scroll was called "scrolling acceleration". */
+static int hidpp10_enable_scrolling_acceleration(struct hidpp_device *hidpp_dev)
+{
+	return hidpp10_set_register_bit(hidpp_dev, HIDPP_REG_FEATURES, 0, 6);
 }
 
 #define HIDPP_REG_BATTERY_STATUS			0x07
-- 
2.19.2

^ permalink raw reply related

* [PATCH v3 5/8] HID: logitech-hidpp: fix typo, hiddpp to hidpp
From: Peter Hutterer @ 2018-12-05  0:42 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Jiri Kosina, Harry Cutts, torvalds,
	Nestor Lopez Casado, linux-kernel, Benjamin Tissoires
In-Reply-To: <20181205004228.10714-1-peter.hutterer@who-t.net>

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
Unchanged since v1

 drivers/hid/hid-logitech-hidpp.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 19cc980eebce..22b37a3844d1 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -1465,7 +1465,7 @@ struct hidpp_ff_work_data {
 	u8 size;
 };
 
-static const signed short hiddpp_ff_effects[] = {
+static const signed short hidpp_ff_effects[] = {
 	FF_CONSTANT,
 	FF_PERIODIC,
 	FF_SINE,
@@ -1480,7 +1480,7 @@ static const signed short hiddpp_ff_effects[] = {
 	-1
 };
 
-static const signed short hiddpp_ff_effects_v2[] = {
+static const signed short hidpp_ff_effects_v2[] = {
 	FF_RAMP,
 	FF_FRICTION,
 	FF_INERTIA,
@@ -1873,11 +1873,11 @@ static int hidpp_ff_init(struct hidpp_device *hidpp, u8 feature_index)
 	version = bcdDevice & 255;
 
 	/* Set supported force feedback capabilities */
-	for (j = 0; hiddpp_ff_effects[j] >= 0; j++)
-		set_bit(hiddpp_ff_effects[j], dev->ffbit);
+	for (j = 0; hidpp_ff_effects[j] >= 0; j++)
+		set_bit(hidpp_ff_effects[j], dev->ffbit);
 	if (version > 1)
-		for (j = 0; hiddpp_ff_effects_v2[j] >= 0; j++)
-			set_bit(hiddpp_ff_effects_v2[j], dev->ffbit);
+		for (j = 0; hidpp_ff_effects_v2[j] >= 0; j++)
+			set_bit(hidpp_ff_effects_v2[j], dev->ffbit);
 
 	/* Read number of slots available in device */
 	error = hidpp_send_fap_command_sync(hidpp, feature_index,
-- 
2.19.2

^ permalink raw reply related

* [PATCH v3 4/8] HID: input: use the Resolution Multiplier for high-resolution scrolling
From: Peter Hutterer @ 2018-12-05  0:42 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Jiri Kosina, Harry Cutts, torvalds,
	Nestor Lopez Casado, linux-kernel, Benjamin Tissoires
In-Reply-To: <20181205004228.10714-1-peter.hutterer@who-t.net>

Windows uses a magic number of 120 for a wheel click. High-resolution
scroll wheels are supposed to use a fraction of 120 to signal smaller
scroll steps. This is implemented by the Resolution Multiplier in the
device itself.

If the multiplier is present in the report descriptor, set it to the
logical max and then use the resolution multiplier to calculate the
high-resolution events. This is the recommendation by Microsoft, see
http://msdn.microsoft.com/en-us/windows/hardware/gg487477.aspx

Note that all mice encountered so far have a logical min/max of 0/1, so
it's a binary "yes or no" to high-res scrolling anyway.

To make userspace simpler, always enable the REL_WHEEL_HI_RES bit. Where
the device doesn't support high-resolution scrolling, the value for the
high-res data will simply be a multiple of 120 every time. For userspace,
if REL_WHEEL_HI_RES is available that is the one to be used.

Potential side-effect: a device with a Resolution Multiplier applying to
other Input items will have those items set to the logical max as well.
This cannot easily be worked around but it is doubtful such devices exist.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
Changes since v1:
- drop the wheel factor and calculate the hi-res value as the event comes
  in. This fixes the issue with a multiplier of 16, makes the code simpler
  too because we don't have to refresh anything after setting the
  multiplier.

Changes since v2:
- unchanged

 drivers/hid/hid-input.c | 108 ++++++++++++++++++++++++++++++++++++++--
 include/linux/hid.h     |   3 ++
 2 files changed, 108 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index d6fab5798487..59a5608b8dc0 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -712,7 +712,15 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 				map_abs_clear(usage->hid & 0xf);
 			break;
 
-		case HID_GD_SLIDER: case HID_GD_DIAL: case HID_GD_WHEEL:
+		case HID_GD_WHEEL:
+			if (field->flags & HID_MAIN_ITEM_RELATIVE) {
+				set_bit(REL_WHEEL, input->relbit);
+				map_rel(REL_WHEEL_HI_RES);
+			} else {
+				map_abs(usage->hid & 0xf);
+			}
+			break;
+		case HID_GD_SLIDER: case HID_GD_DIAL:
 			if (field->flags & HID_MAIN_ITEM_RELATIVE)
 				map_rel(usage->hid & 0xf);
 			else
@@ -1012,7 +1020,10 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 		case 0x22f: map_key_clear(KEY_ZOOMRESET);	break;
 		case 0x233: map_key_clear(KEY_SCROLLUP);	break;
 		case 0x234: map_key_clear(KEY_SCROLLDOWN);	break;
-		case 0x238: map_rel(REL_HWHEEL);		break;
+		case 0x238: /* AC Pan */
+			set_bit(REL_HWHEEL, input->relbit);
+			map_rel(REL_HWHEEL_HI_RES);
+			break;
 		case 0x23d: map_key_clear(KEY_EDIT);		break;
 		case 0x25f: map_key_clear(KEY_CANCEL);		break;
 		case 0x269: map_key_clear(KEY_INSERT);		break;
@@ -1200,6 +1211,38 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 
 }
 
+static void hidinput_handle_scroll(struct hid_usage *usage,
+				   struct input_dev *input,
+				   __s32 value)
+{
+	int code;
+	int hi_res, lo_res;
+
+	if (value == 0)
+		return;
+
+	if (usage->code == REL_WHEEL_HI_RES)
+		code = REL_WHEEL;
+	else
+		code = REL_HWHEEL;
+
+	/*
+	 * Windows reports one wheel click as value 120. Where a high-res
+	 * scroll wheel is present, a fraction of 120 is reported instead.
+	 * Our REL_WHEEL_HI_RES axis does the same because all HW must
+	 * adhere to the 120 expectation.
+	 */
+	hi_res = value * 120/usage->resolution_multiplier;
+
+	usage->wheel_accumulated += hi_res;
+	lo_res = usage->wheel_accumulated/120;
+	if (lo_res)
+		usage->wheel_accumulated -= lo_res * 120;
+
+	input_event(input, EV_REL, code, lo_res);
+	input_event(input, EV_REL, usage->code, hi_res);
+}
+
 void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct hid_usage *usage, __s32 value)
 {
 	struct input_dev *input;
@@ -1262,6 +1305,12 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 	if ((usage->type == EV_KEY) && (usage->code == 0)) /* Key 0 is "unassigned", not KEY_UNKNOWN */
 		return;
 
+	if ((usage->type == EV_REL) && (usage->code == REL_WHEEL_HI_RES ||
+					usage->code == REL_HWHEEL_HI_RES)) {
+		hidinput_handle_scroll(usage, input, value);
+		return;
+	}
+
 	if ((usage->type == EV_ABS) && (field->flags & HID_MAIN_ITEM_RELATIVE) &&
 			(usage->code == ABS_VOLUME)) {
 		int count = abs(value);
@@ -1489,6 +1538,58 @@ static void hidinput_close(struct input_dev *dev)
 	hid_hw_close(hid);
 }
 
+static void hidinput_change_resolution_multipliers(struct hid_device *hid)
+{
+	struct hid_report_enum *rep_enum;
+	struct hid_report *rep;
+	struct hid_usage *usage;
+	int i, j;
+
+	rep_enum = &hid->report_enum[HID_FEATURE_REPORT];
+	list_for_each_entry(rep, &rep_enum->report_list, list) {
+		bool update_needed = false;
+
+		if (rep->maxfield == 0)
+			continue;
+
+		/*
+		 * If we have more than one feature within this report we
+		 * need to fill in the bits from the others before we can
+		 * overwrite the ones for the Resolution Multiplier.
+		 */
+		if (rep->maxfield > 1) {
+			hid_hw_request(hid, rep, HID_REQ_GET_REPORT);
+			hid_hw_wait(hid);
+		}
+
+		for (i = 0; i < rep->maxfield; i++) {
+			__s32 logical_max = rep->field[i]->logical_maximum;
+
+			/* There is no good reason for a Resolution
+			 * Multiplier to have a count other than 1.
+			 * Ignore that case.
+			 */
+			if (rep->field[i]->report_count != 1)
+				continue;
+
+			for (j = 0; j < rep->field[i]->maxusage; j++) {
+				usage = &rep->field[i]->usage[j];
+
+				if (usage->hid != HID_GD_RESOLUTION_MULTIPLIER)
+					continue;
+
+				*rep->field[i]->value = logical_max;
+				update_needed = true;
+			}
+		}
+		if (update_needed)
+			hid_hw_request(hid, rep, HID_REQ_SET_REPORT);
+	}
+
+	/* refresh our structs */
+	hid_setup_resolution_multiplier(hid);
+}
+
 static void report_features(struct hid_device *hid)
 {
 	struct hid_driver *drv = hid->driver;
@@ -1782,6 +1883,8 @@ int hidinput_connect(struct hid_device *hid, unsigned int force)
 		}
 	}
 
+	hidinput_change_resolution_multipliers(hid);
+
 	list_for_each_entry_safe(hidinput, next, &hid->inputs, list) {
 		if (drv->input_configured &&
 		    drv->input_configured(hid, hidinput))
@@ -1840,4 +1943,3 @@ void hidinput_disconnect(struct hid_device *hid)
 	cancel_work_sync(&hid->led_work);
 }
 EXPORT_SYMBOL_GPL(hidinput_disconnect);
-
diff --git a/include/linux/hid.h b/include/linux/hid.h
index fd8d860365a4..93db548f8761 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -233,6 +233,7 @@ struct hid_item {
 #define HID_DC_BATTERYSTRENGTH	0x00060020
 
 #define HID_CP_CONSUMER_CONTROL	0x000c0001
+#define HID_CP_AC_PAN		0x000c0238
 
 #define HID_DG_DIGITIZER	0x000d0001
 #define HID_DG_PEN		0x000d0002
@@ -441,11 +442,13 @@ struct hid_usage {
 	__s8	  resolution_multiplier;/* Effective Resolution Multiplier
 					   (HUT v1.12, 4.3.1), default: 1 */
 	/* hidinput data */
+	__s8	  wheel_factor;		/* 120/resolution_multiplier */
 	__u16     code;			/* input driver code */
 	__u8      type;			/* input driver type */
 	__s8	  hat_min;		/* hat switch fun */
 	__s8	  hat_max;		/* ditto */
 	__s8	  hat_dir;		/* ditto */
+	__s16	  wheel_accumulated;	/* hi-res wheel */
 };
 
 struct hid_input;
-- 
2.19.2

^ permalink raw reply related

* [PATCH v3 3/8] HID: core: process the Resolution Multiplier
From: Peter Hutterer @ 2018-12-05  0:42 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Jiri Kosina, Harry Cutts, torvalds,
	Nestor Lopez Casado, linux-kernel, Benjamin Tissoires
In-Reply-To: <20181205004228.10714-1-peter.hutterer@who-t.net>

The Resolution Multiplier is a feature report that modifies the value of
Usages within the same Logical Collection. If the multiplier is set to
anything but 1, the hardware reports (value * multiplier) for the same amount
of physical movement, i.e. the value we receive in the kernel is
pre-multiplied.

The hardware may either send a single (value * multiplier), or by sending
multiplier as many reports with the same value, or a combination of these two
options. For example, when the Microsoft Sculpt Ergonomic mouse Resolution
Multiplier is set to 12, the Wheel sends out 12 for every detent but AC Pan
sends out a value of 3 at 4 times the frequency.

The effective multiplier is based on the physical min/max of the multiplier
field, a logical min/max of [0,1] with a physical min/max of [1,8] means the
multiplier is either 1 or 8.

The Resolution Multiplier was introduced for high-resolution scrolling in
Windows Vista and is commonly used on Microsoft mice.

The recommendation for the Resolution Multiplier is to default to 1 for
backwards compatibility. This patch adds an arbitrary upper limit at 255. The
only known use case for the Resolution Multiplier is for scroll wheels where the
multiplier has to be a fraction of 120 to work with Windows.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
Changes since v1, v2:
- expanded the commit message with more detail 

 drivers/hid/hid-core.c | 170 +++++++++++++++++++++++++++++++++++++++++
 include/linux/hid.h    |   5 ++
 2 files changed, 175 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 43d488a45120..f41d5fe51abe 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -294,6 +294,7 @@ static int hid_add_field(struct hid_parser *parser, unsigned report_type, unsign
 		field->usage[i].collection_index =
 			parser->local.collection_index[j];
 		field->usage[i].usage_index = i;
+		field->usage[i].resolution_multiplier = 1;
 	}
 
 	field->maxusage = usages;
@@ -947,6 +948,167 @@ struct hid_report *hid_validate_values(struct hid_device *hid,
 }
 EXPORT_SYMBOL_GPL(hid_validate_values);
 
+static int hid_calculate_multiplier(struct hid_device *hid,
+				     struct hid_field *multiplier)
+{
+	int m;
+	__s32 v = *multiplier->value;
+	__s32 lmin = multiplier->logical_minimum;
+	__s32 lmax = multiplier->logical_maximum;
+	__s32 pmin = multiplier->physical_minimum;
+	__s32 pmax = multiplier->physical_maximum;
+
+	/*
+	 * "Because OS implementations will generally divide the control's
+	 * reported count by the Effective Resolution Multiplier, designers
+	 * should take care not to establish a potential Effective
+	 * Resolution Multiplier of zero."
+	 * HID Usage Table, v1.12, Section 4.3.1, p31
+	 */
+	if (lmax - lmin == 0)
+		return 1;
+	/*
+	 * Handling the unit exponent is left as an exercise to whoever
+	 * finds a device where that exponent is not 0.
+	 */
+	m = ((v - lmin)/(lmax - lmin) * (pmax - pmin) + pmin);
+	if (unlikely(multiplier->unit_exponent != 0)) {
+		hid_warn(hid,
+			 "unsupported Resolution Multiplier unit exponent %d\n",
+			 multiplier->unit_exponent);
+	}
+
+	/* There are no devices with an effective multiplier > 255 */
+	if (unlikely(m == 0 || m > 255 || m < -255)) {
+		hid_warn(hid, "unsupported Resolution Multiplier %d\n", m);
+		m = 1;
+	}
+
+	return m;
+}
+
+static void hid_apply_multiplier_to_field(struct hid_device *hid,
+					  struct hid_field *field,
+					  struct hid_collection *multiplier_collection,
+					  int effective_multiplier)
+{
+	struct hid_collection *collection;
+	struct hid_usage *usage;
+	int i;
+
+	/*
+	 * If multiplier_collection is NULL, the multiplier applies
+	 * to all fields in the report.
+	 * Otherwise, it is the Logical Collection the multiplier applies to
+	 * but our field may be in a subcollection of that collection.
+	 */
+	for (i = 0; i < field->maxusage; i++) {
+		usage = &field->usage[i];
+
+		collection = &hid->collection[usage->collection_index];
+		while (collection && collection != multiplier_collection)
+			collection = collection->parent;
+
+		if (collection || multiplier_collection == NULL)
+			usage->resolution_multiplier = effective_multiplier;
+
+	}
+}
+
+static void hid_apply_multiplier(struct hid_device *hid,
+				 struct hid_field *multiplier)
+{
+	struct hid_report_enum *rep_enum;
+	struct hid_report *rep;
+	struct hid_field *field;
+	struct hid_collection *multiplier_collection;
+	int effective_multiplier;
+	int i;
+
+	/*
+	 * "The Resolution Multiplier control must be contained in the same
+	 * Logical Collection as the control(s) to which it is to be applied.
+	 * If no Resolution Multiplier is defined, then the Resolution
+	 * Multiplier defaults to 1.  If more than one control exists in a
+	 * Logical Collection, the Resolution Multiplier is associated with
+	 * all controls in the collection. If no Logical Collection is
+	 * defined, the Resolution Multiplier is associated with all
+	 * controls in the report."
+	 * HID Usage Table, v1.12, Section 4.3.1, p30
+	 *
+	 * Thus, search from the current collection upwards until we find a
+	 * logical collection. Then search all fields for that same parent
+	 * collection. Those are the fields the multiplier applies to.
+	 *
+	 * If we have more than one multiplier, it will overwrite the
+	 * applicable fields later.
+	 */
+	multiplier_collection = &hid->collection[multiplier->usage->collection_index];
+	while (multiplier_collection &&
+	       multiplier_collection->type != HID_COLLECTION_LOGICAL)
+		multiplier_collection = multiplier_collection->parent;
+
+	effective_multiplier = hid_calculate_multiplier(hid, multiplier);
+
+	rep_enum = &hid->report_enum[HID_INPUT_REPORT];
+	list_for_each_entry(rep, &rep_enum->report_list, list) {
+		for (i = 0; i < rep->maxfield; i++) {
+			field = rep->field[i];
+			hid_apply_multiplier_to_field(hid, field,
+						      multiplier_collection,
+						      effective_multiplier);
+		}
+	}
+}
+
+/*
+ * hid_setup_resolution_multiplier - set up all resolution multipliers
+ *
+ * @device: hid device
+ *
+ * Search for all Resolution Multiplier Feature Reports and apply their
+ * value to all matching Input items. This only updates the internal struct
+ * fields.
+ *
+ * The Resolution Multiplier is applied by the hardware. If the multiplier
+ * is anything other than 1, the hardware will send pre-multiplied events
+ * so that the same physical interaction generates an accumulated
+ *	accumulated_value = value * * multiplier
+ * This may be achieved by sending
+ * - "value * multiplier" for each event, or
+ * - "value" but "multiplier" times as frequently, or
+ * - a combination of the above
+ * The only guarantee is that the same physical interaction always generates
+ * an accumulated 'value * multiplier'.
+ *
+ * This function must be called before any event processing and after
+ * any SetRequest to the Resolution Multiplier.
+ */
+void hid_setup_resolution_multiplier(struct hid_device *hid)
+{
+	struct hid_report_enum *rep_enum;
+	struct hid_report *rep;
+	struct hid_usage *usage;
+	int i, j;
+
+	rep_enum = &hid->report_enum[HID_FEATURE_REPORT];
+	list_for_each_entry(rep, &rep_enum->report_list, list) {
+		for (i = 0; i < rep->maxfield; i++) {
+			/* Ignore if report count is out of bounds. */
+			if (rep->field[i]->report_count < 1)
+				continue;
+
+			for (j = 0; j < rep->field[i]->maxusage; j++) {
+				usage = &rep->field[i]->usage[j];
+				if (usage->hid == HID_GD_RESOLUTION_MULTIPLIER)
+					hid_apply_multiplier(hid,
+							     rep->field[i]);
+			}
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(hid_setup_resolution_multiplier);
+
 /**
  * hid_open_report - open a driver-specific device report
  *
@@ -1043,9 +1205,17 @@ int hid_open_report(struct hid_device *device)
 				hid_err(device, "unbalanced delimiter at end of report description\n");
 				goto err;
 			}
+
+			/*
+			 * fetch initial values in case the device's
+			 * default multiplier isn't the recommended 1
+			 */
+			hid_setup_resolution_multiplier(device);
+
 			kfree(parser->collection_stack);
 			vfree(parser);
 			device->status |= HID_STAT_PARSED;
+
 			return 0;
 		}
 	}
diff --git a/include/linux/hid.h b/include/linux/hid.h
index fdfda898656c..fd8d860365a4 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -219,6 +219,7 @@ struct hid_item {
 #define HID_GD_VBRZ		0x00010045
 #define HID_GD_VNO		0x00010046
 #define HID_GD_FEATURE		0x00010047
+#define HID_GD_RESOLUTION_MULTIPLIER	0x00010048
 #define HID_GD_SYSTEM_CONTROL	0x00010080
 #define HID_GD_UP		0x00010090
 #define HID_GD_DOWN		0x00010091
@@ -437,6 +438,8 @@ struct hid_usage {
 	unsigned  hid;			/* hid usage code */
 	unsigned  collection_index;	/* index into collection array */
 	unsigned  usage_index;		/* index into usage array */
+	__s8	  resolution_multiplier;/* Effective Resolution Multiplier
+					   (HUT v1.12, 4.3.1), default: 1 */
 	/* hidinput data */
 	__u16     code;			/* input driver code */
 	__u8      type;			/* input driver type */
@@ -894,6 +897,8 @@ struct hid_report *hid_validate_values(struct hid_device *hid,
 				       unsigned int type, unsigned int id,
 				       unsigned int field_index,
 				       unsigned int report_counts);
+
+void hid_setup_resolution_multiplier(struct hid_device *hid);
 int hid_open_report(struct hid_device *device);
 int hid_check_keys_pressed(struct hid_device *hid);
 int hid_connect(struct hid_device *hid, unsigned int connect_mask);
-- 
2.19.2

^ permalink raw reply related

* [PATCH v3 2/8] HID: core: store the collections as a basic tree
From: Peter Hutterer @ 2018-12-05  0:42 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Jiri Kosina, Harry Cutts, torvalds,
	Nestor Lopez Casado, linux-kernel, Benjamin Tissoires
In-Reply-To: <20181205004228.10714-1-peter.hutterer@who-t.net>

For each collection parsed, store a pointer to the parent collection
(if any). This makes it a lot easier to look up which collection(s)
any given item is part of

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
No changes since v1

 drivers/hid/hid-core.c | 4 ++++
 include/linux/hid.h    | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 5bec9244c45b..43d488a45120 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -172,6 +172,8 @@ static int open_collection(struct hid_parser *parser, unsigned type)
 	collection->type = type;
 	collection->usage = usage;
 	collection->level = parser->collection_stack_ptr - 1;
+	collection->parent = parser->active_collection;
+	parser->active_collection = collection;
 
 	if (type == HID_COLLECTION_APPLICATION)
 		parser->device->maxapplication++;
@@ -190,6 +192,8 @@ static int close_collection(struct hid_parser *parser)
 		return -EINVAL;
 	}
 	parser->collection_stack_ptr--;
+	if (parser->active_collection)
+		parser->active_collection = parser->active_collection->parent;
 	return 0;
 }
 
diff --git a/include/linux/hid.h b/include/linux/hid.h
index a355d61940f2..fdfda898656c 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -427,6 +427,7 @@ struct hid_local {
  */
 
 struct hid_collection {
+	struct hid_collection *parent;
 	unsigned type;
 	unsigned usage;
 	unsigned level;
@@ -650,6 +651,7 @@ struct hid_parser {
 	unsigned int         *collection_stack;
 	unsigned int          collection_stack_ptr;
 	unsigned int          collection_stack_size;
+	struct hid_collection *active_collection;
 	struct hid_device    *device;
 	unsigned int          scan_flags;
 };
-- 
2.19.2

^ permalink raw reply related

* [PATCH v3 1/8] Input: add `REL_WHEEL_HI_RES` and `REL_HWHEEL_HI_RES`
From: Peter Hutterer @ 2018-12-05  0:42 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Jiri Kosina, Harry Cutts, torvalds,
	Nestor Lopez Casado, linux-kernel, Benjamin Tissoires
In-Reply-To: <20181205004228.10714-1-peter.hutterer@who-t.net>

This event code represents scroll reports from high-resolution wheels and
is modelled after the approach Windows uses. The value 120 is one detent
(wheel click) of movement. Mice with higher-resolution scrolling can send
fractions of 120 which must be accumulated in userspace. Userspace can either
wait for a full 120 to accumulate or scroll by fractions of one logical scroll
movement as the events come in. 120 was picked as magic number because it has
a high number of integer fractions that can be used by high-resolution wheels.

For more information see
https://docs.microsoft.com/en-us/previous-versions/windows/hardware/design/dn613912(v=vs.85)

These new axes obsolete REL_WHEEL and REL_HWHEEL. The legacy axes are emulated
by the kernel but the most accurate (and most granular) data is available
through the new axes.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
No changes since v1

 Documentation/input/event-codes.rst    | 21 ++++++++++++++++++++-
 include/uapi/linux/input-event-codes.h |  2 ++
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
index a8c0873beb95..b24b5343f5eb 100644
--- a/Documentation/input/event-codes.rst
+++ b/Documentation/input/event-codes.rst
@@ -190,7 +190,26 @@ A few EV_REL codes have special meanings:
 * REL_WHEEL, REL_HWHEEL:
 
   - These codes are used for vertical and horizontal scroll wheels,
-    respectively.
+    respectively. The value is the number of detents moved on the wheel, the
+    physical size of which varies by device. For high-resolution wheels
+    this may be an approximation based on the high-resolution scroll events,
+    see REL_WHEEL_HI_RES. These event codes are legacy codes and
+    REL_WHEEL_HI_RES and REL_HWHEEL_HI_RES should be preferred where
+    available.
+
+* REL_WHEEL_HI_RES, REL_HWHEEL_HI_RES:
+
+  - High-resolution scroll wheel data. The accumulated value 120 represents
+    movement by one detent. For devices that do not provide high-resolution
+    scrolling, the value is always a multiple of 120. For devices with
+    high-resolution scrolling, the value may be a fraction of 120.
+
+    If a vertical scroll wheel supports high-resolution scrolling, this code
+    will be emitted in addition to REL_WHEEL or REL_HWHEEL. The REL_WHEEL
+    and REL_HWHEEL may be an approximation based on the high-resolution
+    scroll events. There is no guarantee that the high-resolution data
+    is a multiple of 120 at the time of an emulated REL_WHEEL or REL_HWHEEL
+    event.
 
 EV_ABS
 ------
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 3eb5a4c3d60a..265ef2028660 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -716,6 +716,8 @@
  * the situation described above.
  */
 #define REL_RESERVED		0x0a
+#define REL_WHEEL_HI_RES	0x0b
+#define REL_HWHEEL_HI_RES	0x0c
 #define REL_MAX			0x0f
 #define REL_CNT			(REL_MAX+1)
 
-- 
2.19.2

^ permalink raw reply related

* [PATCH v3 0/8] HID: MS and Logitech high-resolution scroll wheel support
From: Peter Hutterer @ 2018-12-05  0:42 UTC (permalink / raw)
  To: linux-input
  Cc: Dmitry Torokhov, Jiri Kosina, Harry Cutts, torvalds,
	Nestor Lopez Casado, linux-kernel, Benjamin Tissoires

A full explanation of why and what is in the v1, v2 patch thread here:
https://lkml.org/lkml/2018/11/22/625

v3 adds a better commit messages, m560 REL_HWHEEL_HI_RES support and a patch 
moved in the ordering. This is a full patch sequence because Benjamin's
magic scripts struggle with singular updates ;)

hid-tools patches to add the tests:
https://gitlab.freedesktop.org/libevdev/hid-tools/merge_requests/12

libinput support (unmerged):
https://gitlab.freedesktop.org/whot/libinput/tree/wip/hi-res-scrolling
If you want to 'feel' how it works, run sudo ./builddir/libinput-debug-gui
and watch the green/red scroll bars. On a device with hi-res scrolling the
green bar (scroll fractions) will move smoother than the red bar (wheel
clicks).

Tested with:
- Microsoft Comfort Optical Mouse 3000
- Microsoft Sculpt Ergonomic Mouse
- Microsoft Wireless Mobile Mouse 4000
- Logitech MX Anywhere 2S

And a few other mice that don't have that feature, so the testing was of
limited excitement.

Cheers,
  Peter

Harry Cutts (3):
      HID: logitech: Add function to enable HID++ 1.0 "scrolling acceleration"
      HID: logitech: Enable high-resolution scrolling on Logitech mice
      HID: logitech: Use LDJ_DEVICE macro for existing Logitech mice

Peter Hutterer (5):
      Input: add `REL_WHEEL_HI_RES` and `REL_HWHEEL_HI_RES`
      HID: core: store the collections as a basic tree
      HID: core: process the Resolution Multiplier
      HID: input: use the Resolution Multiplier for high-resolution scrolling
      HID: logitech-hidpp: fix typo, hiddpp to hidpp

 Documentation/input/event-codes.rst    |  21 +++-
 drivers/hid/hid-core.c                 | 174 +++++++++++++++++++++++++++++++++
 drivers/hid/hid-input.c                | 108 +++++++++++++++++++-
 drivers/hid/hid-logitech-hidpp.c       | 375 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 include/linux/hid.h                    |  10 ++
 include/uapi/linux/input-event-codes.h |   2 +
 6 files changed, 651 insertions(+), 39 deletions(-)

^ permalink raw reply

* Re: [PATCH] dt-bindings: ti-tsc-adc: Add new compatible for AM654 SoCs
From: Rob Herring @ 2018-12-04 23:16 UTC (permalink / raw)
  Cc: Dmitry Torokhov, Rob Herring, Lee Jones, Vignesh R, linux-input,
	linux-omap, devicetree, linux-kernel
In-Reply-To: <20181119094509.22542-1-vigneshr@ti.com>

On Mon, 19 Nov 2018 15:15:09 +0530, Vignesh R wrote:
> AM654 SoCs has ADC IP which is similar to AM335x. Add new compatible to
> handle AM654 SoCs. Also, it seems that existing compatible strings used
> in the kernel DTs were never documented. So, document them now.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---
>  .../devicetree/bindings/input/touchscreen/ti-tsc-adc.txt  | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: [PATCH 2/2] Input: omap-keypad: Fix idle configration to not block SoC idle states
From: Dmitry Torokhov @ 2018-12-04 19:15 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-input, linux-kernel, linux-omap, Axel Haslam, Illia Smyrnov,
	Marcel Partap, Merlijn Wajer, Michael Scott, NeKit, Pavel Machek,
	Sebastian Reichel
In-Reply-To: <20181204190932.GD6707@atomide.com>

On December 4, 2018 11:09:32 AM PST, Tony Lindgren <tony@atomide.com> wrote:
>Hi,
>
>* Dmitry Torokhov <dmitry.torokhov@gmail.com> [181204 04:00]:
>> Hi Tony,
>> 
>> On Mon, Dec 03, 2018 at 03:12:51PM -0800, Tony Lindgren wrote:
>> > 
>> > With PM enabled, I noticed that pressing a key on the droid4
>keyboard will
>> > block deeper idle states for the SoC. Looks like we can fix this by
>> > managing the idle register to gether with the interrupt similar to
>what
>> > we already do for the GPIO controller.
>> 
>> Can you show me where exactly we are doing this? I can't seem to find
>> the matching code.
>
>With your change it now becomes the fix, and we're just missing the
>clearing of the OMAP4_KBD_WAKEUPENABLE register in omap4_keypad_close.
>
>Does the following minimal version with updated comments make
>more sense now?

Awesome, thank you Tony.

>
>Regards,
>
>Tony
>
>8< --------------
>From tony Mon Sep 17 00:00:00 2001
>From: Tony Lindgren <tony@atomide.com>
>Date: Tue, 4 Dec 2018 11:07:56 -0800
>Subject: [PATCH] Input: omap-keypad: Fix idle configration to not block
> SoC idle states
>
>With PM enabled, I noticed that pressing a key on the droid4 keyboard
>will
>block deeper idle states for the SoC. Let's fix this by using
>IRQF_ONESHOT
>and stop constantly toggling the device OMAP4_KBD_IRQENABLE register as
>suggested by Dmitry Torokhov <dmitry.torokhov@gmail.com>.
>
>From the hardware point of view, looks like we need to manage the
>registers
>for OMAP4_KBD_IRQENABLE and OMAP4_KBD_WAKEUPENABLE together to avoid
>blocking deeper SoC idle states. And with toggling of
>OMAP4_KBD_IRQENABLE
>register now gone with IRQF_ONESHOT, also the SoC idle state problem is
>gone during runtime. We still also need to clear OMAP4_KBD_WAKEUPENABLE
>in
>omap4_keypad_close() though to pair it with omap4_keypad_open() to
>prevent
>blocking deeper SoC idle states after rmmod omap4-keypad.
>
>Cc: Axel Haslam <axelhaslam@ti.com>
>Cc: Illia Smyrnov <illia.smyrnov@ti.com>
>Cc: Marcel Partap <mpartap@gmx.net>
>Cc: Merlijn Wajer <merlijn@wizzup.org>
>Cc: Michael Scott <hashcode0f@gmail.com>
>Cc: NeKit <nekit1000@gmail.com>
>Cc: Pavel Machek <pavel@ucw.cz>
>Cc: Sebastian Reichel <sre@kernel.org>
>Reported-by: Pavel Machek <pavel@ucw.cz>
>Signed-off-by: Tony Lindgren <tony@atomide.com>
>---
> drivers/input/keyboard/omap4-keypad.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/input/keyboard/omap4-keypad.c
>b/drivers/input/keyboard/omap4-keypad.c
>--- a/drivers/input/keyboard/omap4-keypad.c
>+++ b/drivers/input/keyboard/omap4-keypad.c
>@@ -126,12 +126,8 @@ static irqreturn_t omap4_keypad_irq_handler(int
>irq, void *dev_id)
> {
> 	struct omap4_keypad *keypad_data = dev_id;
> 
>-	if (kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)) {
>-		/* Disable interrupts */
>-		kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
>-				 OMAP4_VAL_IRQDISABLE);
>+	if (kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS))
> 		return IRQ_WAKE_THREAD;
>-	}
> 
> 	return IRQ_NONE;
> }
>@@ -173,11 +169,6 @@ static irqreturn_t omap4_keypad_irq_thread_fn(int
>irq, void *dev_id)
> 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
> 			 kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
> 
>-	/* enable interrupts */
>-	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
>-		OMAP4_DEF_IRQENABLE_EVENTEN |
>-				OMAP4_DEF_IRQENABLE_LONGKEY);
>-
> 	return IRQ_HANDLED;
> }
> 
>@@ -214,9 +205,10 @@ static void omap4_keypad_close(struct input_dev
>*input)
> 
> 	disable_irq(keypad_data->irq);
> 
>-	/* Disable interrupts */
>+	/* Disable interrupts and wake-up events */
> 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
> 			 OMAP4_VAL_IRQDISABLE);
>+	kbd_writel(keypad_data, OMAP4_KBD_WAKEUPENABLE, 0);
> 
> 	/* clear pending interrupts */
> 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
>@@ -365,7 +357,7 @@ static int omap4_keypad_probe(struct
>platform_device *pdev)
> 	}
> 
>	error = request_threaded_irq(keypad_data->irq,
>omap4_keypad_irq_handler,
>-				     omap4_keypad_irq_thread_fn, 0,
>+				     omap4_keypad_irq_thread_fn, IRQF_ONESHOT,
> 				     "omap4-keypad", keypad_data);
> 	if (error) {
> 		dev_err(&pdev->dev, "failed to register interrupt\n");



-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 2/2] Input: omap-keypad: Fix idle configration to not block SoC idle states
From: Tony Lindgren @ 2018-12-04 19:09 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, linux-kernel, linux-omap, Axel Haslam, Illia Smyrnov,
	Marcel Partap, Merlijn Wajer, Michael Scott, NeKit, Pavel Machek,
	Sebastian Reichel
In-Reply-To: <20181204040001.GA239923@dtor-ws>

Hi,

* Dmitry Torokhov <dmitry.torokhov@gmail.com> [181204 04:00]:
> Hi Tony,
> 
> On Mon, Dec 03, 2018 at 03:12:51PM -0800, Tony Lindgren wrote:
> > 
> > With PM enabled, I noticed that pressing a key on the droid4 keyboard will
> > block deeper idle states for the SoC. Looks like we can fix this by
> > managing the idle register to gether with the interrupt similar to what
> > we already do for the GPIO controller.
> 
> Can you show me where exactly we are doing this? I can't seem to find
> the matching code.

With your change it now becomes the fix, and we're just missing the
clearing of the OMAP4_KBD_WAKEUPENABLE register in omap4_keypad_close.

Does the following minimal version with updated comments make
more sense now?

Regards,

Tony

8< --------------
>From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Tue, 4 Dec 2018 11:07:56 -0800
Subject: [PATCH] Input: omap-keypad: Fix idle configration to not block
 SoC idle states

With PM enabled, I noticed that pressing a key on the droid4 keyboard will
block deeper idle states for the SoC. Let's fix this by using IRQF_ONESHOT
and stop constantly toggling the device OMAP4_KBD_IRQENABLE register as
suggested by Dmitry Torokhov <dmitry.torokhov@gmail.com>.

>From the hardware point of view, looks like we need to manage the registers
for OMAP4_KBD_IRQENABLE and OMAP4_KBD_WAKEUPENABLE together to avoid
blocking deeper SoC idle states. And with toggling of OMAP4_KBD_IRQENABLE
register now gone with IRQF_ONESHOT, also the SoC idle state problem is
gone during runtime. We still also need to clear OMAP4_KBD_WAKEUPENABLE in
omap4_keypad_close() though to pair it with omap4_keypad_open() to prevent
blocking deeper SoC idle states after rmmod omap4-keypad.

Cc: Axel Haslam <axelhaslam@ti.com>
Cc: Illia Smyrnov <illia.smyrnov@ti.com>
Cc: Marcel Partap <mpartap@gmx.net>
Cc: Merlijn Wajer <merlijn@wizzup.org>
Cc: Michael Scott <hashcode0f@gmail.com>
Cc: NeKit <nekit1000@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Sebastian Reichel <sre@kernel.org>
Reported-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/input/keyboard/omap4-keypad.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -126,12 +126,8 @@ static irqreturn_t omap4_keypad_irq_handler(int irq, void *dev_id)
 {
 	struct omap4_keypad *keypad_data = dev_id;
 
-	if (kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)) {
-		/* Disable interrupts */
-		kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
-				 OMAP4_VAL_IRQDISABLE);
+	if (kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS))
 		return IRQ_WAKE_THREAD;
-	}
 
 	return IRQ_NONE;
 }
@@ -173,11 +169,6 @@ static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id)
 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
 			 kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
 
-	/* enable interrupts */
-	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
-		OMAP4_DEF_IRQENABLE_EVENTEN |
-				OMAP4_DEF_IRQENABLE_LONGKEY);
-
 	return IRQ_HANDLED;
 }
 
@@ -214,9 +205,10 @@ static void omap4_keypad_close(struct input_dev *input)
 
 	disable_irq(keypad_data->irq);
 
-	/* Disable interrupts */
+	/* Disable interrupts and wake-up events */
 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
 			 OMAP4_VAL_IRQDISABLE);
+	kbd_writel(keypad_data, OMAP4_KBD_WAKEUPENABLE, 0);
 
 	/* clear pending interrupts */
 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
@@ -365,7 +357,7 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 	}
 
 	error = request_threaded_irq(keypad_data->irq, omap4_keypad_irq_handler,
-				     omap4_keypad_irq_thread_fn, 0,
+				     omap4_keypad_irq_thread_fn, IRQF_ONESHOT,
 				     "omap4-keypad", keypad_data);
 	if (error) {
 		dev_err(&pdev->dev, "failed to register interrupt\n");
-- 
2.19.2

^ permalink raw reply

* Re: [PATCH v7 0/7] Introduce STPMIC1 PMIC Driver
From: Pascal PAILLET-LME @ 2018-12-04 15:24 UTC (permalink / raw)
  To: Lee Jones
  Cc: dmitry.torokhov@gmail.com, robh+dt@kernel.org,
	mark.rutland@arm.com, lgirdwood@gmail.com, broonie@kernel.org,
	wim@linux-watchdog.org, linux@roeck-us.net,
	linux-input@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org,
	benjamin.gaignard@linaro.org, eballetbo@gmail.com,
	axel.lin@ingics.com
In-Reply-To: <20181203071145.GA26661@dell>

Hi Lee,

Le 12/03/2018 08:11 AM, Lee Jones a écrit :
> On Fri, 30 Nov 2018, Pascal PAILLET-LME wrote:
>
>> The goal of this patch-set is to propose a driver for the STPMIC1 PMIC from
>> STMicroelectronics.
>> The STPMIC1 regulators supply power to an application processor as well as
>> to external system peripherals such as DDR, Flash memories and system
>> devices. It also features onkey button input and an hardware watchdog.
>> The STPMIC1 is controlled via I2C.
>>
>> Main driver is drivers/mfd/stpmic1 that handle I2C regmap configuration and
>> irqchip. stpmic1_regulator, stpmic1_onkey and stpmic1_wdt need stpmic1 mfd
>> as parent.
>>
>> STPMIC1 MFD and regulator drivers maybe mandatory at boot time.
>>
>> Pascal Paillet (7):
>> changes in v7:
>> * rebase on regul/for-next
>>
>>    dt-bindings: mfd: document stpmic1
>>    mfd: stpmic1: add stpmic1 driver
>>    dt-bindings: input: document stpmic1 pmic onkey
>>    input: stpmic1: add stpmic1 onkey driver
>>    dt-bindings: watchdog: document stpmic1 pmic watchdog
>>    watchdog: stpmic1: add stpmic1 watchdog driver
>>    regulator: stpmic1: fix regulator_lock usage
>>
>>   .../devicetree/bindings/input/st,stpmic1-onkey.txt |  28 +++
>>   .../devicetree/bindings/mfd/st,stpmic1.txt         |  61 ++++++
>>   .../bindings/watchdog/st,stpmic1-wdt.txt           |  11 ++
>>   drivers/input/misc/Kconfig                         |  11 ++
>>   drivers/input/misc/Makefile                        |   2 +
>>   drivers/input/misc/stpmic1_onkey.c                 | 198 +++++++++++++++++++
>>   drivers/mfd/Kconfig                                |  16 ++
>>   drivers/mfd/Makefile                               |   1 +
>>   drivers/mfd/stpmic1.c                              | 213 +++++++++++++++++++++
>>   drivers/regulator/stpmic1_regulator.c              |   2 +-
> Is it just Mark you're waiting on now?
It is ok now, Mark has applied the patch.
Evrey thing has been acked now. What is the nex step ?

>>   drivers/watchdog/Kconfig                           |  12 ++
>>   drivers/watchdog/Makefile                          |   1 +
>>   drivers/watchdog/stpmic1_wdt.c                     | 139 ++++++++++++++
>>   include/dt-bindings/mfd/st,stpmic1.h               |  50 +++++
>>   include/linux/mfd/stpmic1.h                        | 212 ++++++++++++++++++++
>>   15 files changed, 956 insertions(+), 1 deletion(-)
>>   create mode 100644 Documentation/devicetree/bindings/input/st,stpmic1-onkey.txt
>>   create mode 100644 Documentation/devicetree/bindings/mfd/st,stpmic1.txt
>>   create mode 100644 Documentation/devicetree/bindings/watchdog/st,stpmic1-wdt.txt
>>   create mode 100644 drivers/input/misc/stpmic1_onkey.c
>>   create mode 100644 drivers/mfd/stpmic1.c
>>   create mode 100644 drivers/watchdog/stpmic1_wdt.c
>>   create mode 100644 include/dt-bindings/mfd/st,stpmic1.h
>>   create mode 100644 include/linux/mfd/stpmic1.h
>>

^ permalink raw reply

* Re: [PATCH] dt-bindings: ti-tsc-adc: Add new compatible for AM654 SoCs
From: Lee Jones @ 2018-12-04  9:54 UTC (permalink / raw)
  To: Vignesh R
  Cc: Dmitry Torokhov, Rob Herring, linux-input, linux-omap, devicetree,
	linux-kernel
In-Reply-To: <20181204095233.GT26661@dell>

On Tue, 04 Dec 2018, Lee Jones wrote:
> On Tue, 04 Dec 2018, Vignesh R wrote:
> > On 19/11/18 3:15 PM, Vignesh R wrote:
> > > AM654 SoCs has ADC IP which is similar to AM335x. Add new compatible to
> > > handle AM654 SoCs. Also, it seems that existing compatible strings used
> > > in the kernel DTs were never documented. So, document them now.
> > > 
> > > Signed-off-by: Vignesh R <vigneshr@ti.com>
> > > ---
> > 
> > Gentle ping on this patch..
> 
> Don't do that.
> 
> If you think your patch has been forgotten, please submit a [RESEND].

As an aside, the subject line is missing a subsystem, which might be
why it was missed by the subsystem maintainer.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH] dt-bindings: ti-tsc-adc: Add new compatible for AM654 SoCs
From: Lee Jones @ 2018-12-04  9:52 UTC (permalink / raw)
  To: Vignesh R
  Cc: Dmitry Torokhov, Rob Herring, linux-input, linux-omap, devicetree,
	linux-kernel
In-Reply-To: <4b42082c-462d-3d94-8ee9-6e7a61f57134@ti.com>

On Tue, 04 Dec 2018, Vignesh R wrote:

> Hi,
> 
> On 19/11/18 3:15 PM, Vignesh R wrote:
> > AM654 SoCs has ADC IP which is similar to AM335x. Add new compatible to
> > handle AM654 SoCs. Also, it seems that existing compatible strings used
> > in the kernel DTs were never documented. So, document them now.
> > 
> > Signed-off-by: Vignesh R <vigneshr@ti.com>
> > ---
> 
> Gentle ping on this patch..

Don't do that.

If you think your patch has been forgotten, please submit a [RESEND].

> >  .../devicetree/bindings/input/touchscreen/ti-tsc-adc.txt  | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> > index b1163bf97146..aad5e34965eb 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> > @@ -2,7 +2,12 @@
> >  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >  
> >  Required properties:
> > +- mfd
> > +	compatible: Should be
> > +		"ti,am3359-tscadc" for AM335x/AM437x SoCs
> > +		"ti,am654-tscadc", "ti,am3359-tscadc" for AM654 SoCs
> >  - child "tsc"
> > +	compatible: Should be "ti,am3359-tsc".
> >  	ti,wires: Wires refer to application modes i.e. 4/5/8 wire touchscreen
> >  		  support on the platform.
> >  	ti,x-plate-resistance: X plate resistance
> > @@ -25,6 +30,9 @@ Required properties:
> >  			AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
> >  			XP  = 0, XN = 1, YP = 2, YN = 3.
> >  - child "adc"
> > +	compatible: Should be
> > +		    "ti,am3359-adc" for AM335x/AM437x SoCs
> > +		    "ti,am654-adc", "ti,am3359-adc" for AM654 SoCs
> >  	ti,adc-channels: List of analog inputs available for ADC.
> >  			 AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
> >  
> > 
> 

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

^ permalink raw reply

* Re: [PATCH] dt-bindings: ti-tsc-adc: Add new compatible for AM654 SoCs
From: Vignesh R @ 2018-12-04  8:30 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring
  Cc: Lee Jones, linux-input, linux-omap, devicetree, linux-kernel
In-Reply-To: <20181119094509.22542-1-vigneshr@ti.com>

Hi,

On 19/11/18 3:15 PM, Vignesh R wrote:
> AM654 SoCs has ADC IP which is similar to AM335x. Add new compatible to
> handle AM654 SoCs. Also, it seems that existing compatible strings used
> in the kernel DTs were never documented. So, document them now.
> 
> Signed-off-by: Vignesh R <vigneshr@ti.com>
> ---

Gentle ping on this patch..

>  .../devicetree/bindings/input/touchscreen/ti-tsc-adc.txt  | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> index b1163bf97146..aad5e34965eb 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/ti-tsc-adc.txt
> @@ -2,7 +2,12 @@
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>  
>  Required properties:
> +- mfd
> +	compatible: Should be
> +		"ti,am3359-tscadc" for AM335x/AM437x SoCs
> +		"ti,am654-tscadc", "ti,am3359-tscadc" for AM654 SoCs
>  - child "tsc"
> +	compatible: Should be "ti,am3359-tsc".
>  	ti,wires: Wires refer to application modes i.e. 4/5/8 wire touchscreen
>  		  support on the platform.
>  	ti,x-plate-resistance: X plate resistance
> @@ -25,6 +30,9 @@ Required properties:
>  			AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
>  			XP  = 0, XN = 1, YP = 2, YN = 3.
>  - child "adc"
> +	compatible: Should be
> +		    "ti,am3359-adc" for AM335x/AM437x SoCs
> +		    "ti,am654-adc", "ti,am3359-adc" for AM654 SoCs
>  	ti,adc-channels: List of analog inputs available for ADC.
>  			 AIN0 = 0, AIN1 = 1 and so on till AIN7 = 7.
>  
> 

-- 
Regards
Vignesh

^ permalink raw reply

* Re: [PATCH 2/2] Input: omap-keypad: Fix idle configration to not block SoC idle states
From: Dmitry Torokhov @ 2018-12-04  4:00 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-input, linux-kernel, linux-omap, Axel Haslam, Illia Smyrnov,
	Marcel Partap, Merlijn Wajer, Michael Scott, NeKit, Pavel Machek,
	Sebastian Reichel
In-Reply-To: <20181203231251.GB39861@atomide.com>

Hi Tony,

On Mon, Dec 03, 2018 at 03:12:51PM -0800, Tony Lindgren wrote:
> 
> With PM enabled, I noticed that pressing a key on the droid4 keyboard will
> block deeper idle states for the SoC. Looks like we can fix this by
> managing the idle register to gether with the interrupt similar to what
> we already do for the GPIO controller.

Can you show me where exactly we are doing this? I can't seem to find
the matching code.

Thanks!

> 
> And there's no need to keep enabling and disabling interrupts and
> wake-up events for normal use if we use IRQF_ONESHOT as suggested by
> Dmitry Torokhov <dmitry.torokhov@gmail.com> so let's do that too.
> 
> Cc: Axel Haslam <axelhaslam@ti.com>
> Cc: Illia Smyrnov <illia.smyrnov@ti.com>
> Cc: Marcel Partap <mpartap@gmx.net>
> Cc: Merlijn Wajer <merlijn@wizzup.org>
> Cc: Michael Scott <hashcode0f@gmail.com>
> Cc: NeKit <nekit1000@gmail.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Sebastian Reichel <sre@kernel.org>
> Reported-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
>  drivers/input/keyboard/omap4-keypad.c | 30 ++++++++++-----------------
>  1 file changed, 11 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
> --- a/drivers/input/keyboard/omap4-keypad.c
> +++ b/drivers/input/keyboard/omap4-keypad.c
> @@ -53,11 +53,12 @@
>  /* OMAP4 bit definitions */
>  #define OMAP4_DEF_IRQENABLE_EVENTEN	BIT(0)
>  #define OMAP4_DEF_IRQENABLE_LONGKEY	BIT(1)
> -#define OMAP4_DEF_WUP_EVENT_ENA		BIT(0)
> -#define OMAP4_DEF_WUP_LONG_KEY_ENA	BIT(1)
>  #define OMAP4_DEF_CTRL_NOSOFTMODE	BIT(1)
>  #define OMAP4_DEF_CTRL_PTV_SHIFT	2
>  
> +#define OMAP4_KBD_IRQ_MASK		(OMAP4_DEF_IRQENABLE_LONGKEY | \
> +					 OMAP4_DEF_IRQENABLE_EVENTEN)
> +
>  /* OMAP4 values */
>  #define OMAP4_VAL_IRQDISABLE		0x0
>  
> @@ -126,12 +127,8 @@ static irqreturn_t omap4_keypad_irq_handler(int irq, void *dev_id)
>  {
>  	struct omap4_keypad *keypad_data = dev_id;
>  
> -	if (kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)) {
> -		/* Disable interrupts */
> -		kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
> -				 OMAP4_VAL_IRQDISABLE);
> +	if (kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS))
>  		return IRQ_WAKE_THREAD;
> -	}
>  
>  	return IRQ_NONE;
>  }
> @@ -173,11 +170,6 @@ static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id)
>  	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
>  			 kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
>  
> -	/* enable interrupts */
> -	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
> -		OMAP4_DEF_IRQENABLE_EVENTEN |
> -				OMAP4_DEF_IRQENABLE_LONGKEY);
> -
>  	return IRQ_HANDLED;
>  }
>  
> @@ -197,11 +189,10 @@ static int omap4_keypad_open(struct input_dev *input)
>  	/* clear pending interrupts */
>  	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
>  			 kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
> -	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
> -			OMAP4_DEF_IRQENABLE_EVENTEN |
> -				OMAP4_DEF_IRQENABLE_LONGKEY);
> -	kbd_writel(keypad_data, OMAP4_KBD_WAKEUPENABLE,
> -			OMAP4_DEF_WUP_EVENT_ENA | OMAP4_DEF_WUP_LONG_KEY_ENA);
> +
> +	/* enable interrupts and wake-up events */
> +	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE, OMAP4_KBD_IRQ_MASK);
> +	kbd_writel(keypad_data, OMAP4_KBD_WAKEUPENABLE, OMAP4_KBD_IRQ_MASK);
>  
>  	enable_irq(keypad_data->irq);
>  
> @@ -214,9 +205,10 @@ static void omap4_keypad_close(struct input_dev *input)
>  
>  	disable_irq(keypad_data->irq);
>  
> -	/* Disable interrupts */
> +	/* Disable interrupts and wake-up events */
>  	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
>  			 OMAP4_VAL_IRQDISABLE);
> +	kbd_writel(keypad_data, OMAP4_KBD_WAKEUPENABLE, 0);
>  
>  	/* clear pending interrupts */
>  	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
> @@ -365,7 +357,7 @@ static int omap4_keypad_probe(struct platform_device *pdev)
>  	}
>  
>  	error = request_threaded_irq(keypad_data->irq, omap4_keypad_irq_handler,
> -				     omap4_keypad_irq_thread_fn, 0,
> +				     omap4_keypad_irq_thread_fn, IRQF_ONESHOT,
>  				     "omap4-keypad", keypad_data);
>  	if (error) {
>  		dev_err(&pdev->dev, "failed to register interrupt\n");
> -- 
> 2.19.2

-- 
Dmitry

^ permalink raw reply

* [git pull] Input updates for v4.20-rc5
From: Dmitry Torokhov @ 2018-12-04  3:37 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, linux-input

Hi Linus,

Please pull from:

	git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus

to receive updates for the input subsystem. Mostly new IDs for
Elan/Synaptics touchpads, plus a few small fixups.

Changelog:
---------

Adam Wong (1):
      Input: elan_i2c - add support for ELAN0621 touchpad

Brian Norris (1):
      Input: cros_ec_keyb - fix button/switch capability reports

Cameron Gutman (1):
      Input: xpad - quirk all PDP Xbox One gamepads

Christian Hoff (1):
      Input: matrix_keypad - check for errors from of_get_named_gpio()

Colin Ian King (1):
      Input: atkbd - clean up indentation issue

Ding Tao (1):
      Input: dt-bindings - fix a typo in file input-reset.txt

Kuninori Morimoto (2):
      Input: migor_ts - convert to SPDX identifiers
      Input: st1232 - convert to SPDX identifiers

Lyude Paul (1):
      Input: synaptics - add PNP ID for ThinkPad P50 to SMBus

Noah Westervelt (1):
      Input: elan_i2c - add ACPI ID for Lenovo IdeaPad 330-15ARR

Patrick Gaskin (1):
      Input: elan_i2c - add ELAN0620 to the ACPI table

Teika Kazura (1):
      Input: synaptics - enable SMBus for HP 15-ay000

Tony Lindgren (1):
      Input: omap-keypad - fix keyboard debounce configuration

Vitaly Kuznetsov (1):
      Input: hyper-v - fix wakeup from suspend-to-idle

Diffstat:
--------

 .../devicetree/bindings/input/input-reset.txt      |  2 +-
 drivers/hid/hid-hyperv.c                           |  2 +-
 drivers/input/joystick/xpad.c                      | 16 ++++++---------
 drivers/input/keyboard/atkbd.c                     |  2 +-
 drivers/input/keyboard/cros_ec_keyb.c              |  3 ++-
 drivers/input/keyboard/matrix_keypad.c             | 23 +++++++++++++---------
 drivers/input/keyboard/omap4-keypad.c              | 18 +++++++++++++----
 drivers/input/mouse/elan_i2c_core.c                |  3 +++
 drivers/input/mouse/synaptics.c                    |  2 ++
 drivers/input/serio/hyperv-keyboard.c              |  2 +-
 drivers/input/touchscreen/migor_ts.c               | 15 +-------------
 drivers/input/touchscreen/st1232.c                 | 12 ++---------
 12 files changed, 48 insertions(+), 52 deletions(-)

Thanks.


-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 2/2] Input: omap-keypad: Fix idle configration to not block SoC idle states
From: Tony Lindgren @ 2018-12-03 23:12 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, linux-kernel, linux-omap, Axel Haslam, Illia Smyrnov,
	Marcel Partap, Merlijn Wajer, Michael Scott, NeKit, Pavel Machek,
	Sebastian Reichel
In-Reply-To: <20181203192351.GB257010@dtor-ws>

Hi,

* Dmitry Torokhov <dmitry.torokhov@gmail.com> [181203 19:24]:
> On Sun, Dec 02, 2018 at 05:29:33PM -0800, Tony Lindgren wrote:
...
> > Note that we now must also disable OMAP4_DEF_IRQENABLE_LONGKEY as it
> > should not be used together with debounce according to the TRM.

The above statement is no longer true, it's left over from an
earlier version of this fix, so will drop it.

> > --- a/drivers/input/keyboard/omap4-keypad.c
> > +++ b/drivers/input/keyboard/omap4-keypad.c
> > @@ -127,9 +128,11 @@ static irqreturn_t omap4_keypad_irq_handler(int irq, void *dev_id)
> >  	struct omap4_keypad *keypad_data = dev_id;
> >  
> >  	if (kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)) {
> > -		/* Disable interrupts */
> > +		/* Disable interrupts and wake-up events */
> >  		kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
> >  				 OMAP4_VAL_IRQDISABLE);
> 
> I wonder, do we need to turn off interrupts at keyboard controller
> level, or we should simply use IRQF_ONESHOT?

Oh that's a good idea and seems to work nicely :)

> > +		kbd_writel(keypad_data, OMAP4_KBD_WAKEUPENABLE, 0);
> 
> So we are saying that disabling wakeup for the time between hard
> interrupt firing, and when interrupt thread runs, makes much difference?
> It is surprising to me... How long does it take to schedule interrupt
> thread?

Nope, you're right. Updated patch below.

Regards,

Tony

8< --------------------
>From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Mon, 3 Dec 2018 14:12:39 -0800
Subject: [PATCH] Input: omap-keypad: Fix idle configration to not block
 SoC idle states

With PM enabled, I noticed that pressing a key on the droid4 keyboard will
block deeper idle states for the SoC. Looks like we can fix this by
managing the idle register to gether with the interrupt similar to what
we already do for the GPIO controller.

And there's no need to keep enabling and disabling interrupts and
wake-up events for normal use if we use IRQF_ONESHOT as suggested by
Dmitry Torokhov <dmitry.torokhov@gmail.com> so let's do that too.

Cc: Axel Haslam <axelhaslam@ti.com>
Cc: Illia Smyrnov <illia.smyrnov@ti.com>
Cc: Marcel Partap <mpartap@gmx.net>
Cc: Merlijn Wajer <merlijn@wizzup.org>
Cc: Michael Scott <hashcode0f@gmail.com>
Cc: NeKit <nekit1000@gmail.com>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Sebastian Reichel <sre@kernel.org>
Reported-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 drivers/input/keyboard/omap4-keypad.c | 30 ++++++++++-----------------
 1 file changed, 11 insertions(+), 19 deletions(-)

diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -53,11 +53,12 @@
 /* OMAP4 bit definitions */
 #define OMAP4_DEF_IRQENABLE_EVENTEN	BIT(0)
 #define OMAP4_DEF_IRQENABLE_LONGKEY	BIT(1)
-#define OMAP4_DEF_WUP_EVENT_ENA		BIT(0)
-#define OMAP4_DEF_WUP_LONG_KEY_ENA	BIT(1)
 #define OMAP4_DEF_CTRL_NOSOFTMODE	BIT(1)
 #define OMAP4_DEF_CTRL_PTV_SHIFT	2
 
+#define OMAP4_KBD_IRQ_MASK		(OMAP4_DEF_IRQENABLE_LONGKEY | \
+					 OMAP4_DEF_IRQENABLE_EVENTEN)
+
 /* OMAP4 values */
 #define OMAP4_VAL_IRQDISABLE		0x0
 
@@ -126,12 +127,8 @@ static irqreturn_t omap4_keypad_irq_handler(int irq, void *dev_id)
 {
 	struct omap4_keypad *keypad_data = dev_id;
 
-	if (kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS)) {
-		/* Disable interrupts */
-		kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
-				 OMAP4_VAL_IRQDISABLE);
+	if (kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS))
 		return IRQ_WAKE_THREAD;
-	}
 
 	return IRQ_NONE;
 }
@@ -173,11 +170,6 @@ static irqreturn_t omap4_keypad_irq_thread_fn(int irq, void *dev_id)
 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
 			 kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
 
-	/* enable interrupts */
-	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
-		OMAP4_DEF_IRQENABLE_EVENTEN |
-				OMAP4_DEF_IRQENABLE_LONGKEY);
-
 	return IRQ_HANDLED;
 }
 
@@ -197,11 +189,10 @@ static int omap4_keypad_open(struct input_dev *input)
 	/* clear pending interrupts */
 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
 			 kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
-	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
-			OMAP4_DEF_IRQENABLE_EVENTEN |
-				OMAP4_DEF_IRQENABLE_LONGKEY);
-	kbd_writel(keypad_data, OMAP4_KBD_WAKEUPENABLE,
-			OMAP4_DEF_WUP_EVENT_ENA | OMAP4_DEF_WUP_LONG_KEY_ENA);
+
+	/* enable interrupts and wake-up events */
+	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE, OMAP4_KBD_IRQ_MASK);
+	kbd_writel(keypad_data, OMAP4_KBD_WAKEUPENABLE, OMAP4_KBD_IRQ_MASK);
 
 	enable_irq(keypad_data->irq);
 
@@ -214,9 +205,10 @@ static void omap4_keypad_close(struct input_dev *input)
 
 	disable_irq(keypad_data->irq);
 
-	/* Disable interrupts */
+	/* Disable interrupts and wake-up events */
 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQENABLE,
 			 OMAP4_VAL_IRQDISABLE);
+	kbd_writel(keypad_data, OMAP4_KBD_WAKEUPENABLE, 0);
 
 	/* clear pending interrupts */
 	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
@@ -365,7 +357,7 @@ static int omap4_keypad_probe(struct platform_device *pdev)
 	}
 
 	error = request_threaded_irq(keypad_data->irq, omap4_keypad_irq_handler,
-				     omap4_keypad_irq_thread_fn, 0,
+				     omap4_keypad_irq_thread_fn, IRQF_ONESHOT,
 				     "omap4-keypad", keypad_data);
 	if (error) {
 		dev_err(&pdev->dev, "failed to register interrupt\n");
-- 
2.19.2

^ permalink raw reply

* Re: [PATCH 1/1] input/mouse: elan_i2c_core: Add ELAN061E ID
From: Dmitry Torokhov @ 2018-12-03 19:28 UTC (permalink / raw)
  To: Harsh Shandilya; +Cc: linux-input, stable
In-Reply-To: <20181118165901.31689-1-harsh@prjkt.io>

Hi Harsh,

On Sun, Nov 18, 2018 at 10:29:01PM +0530, Harsh Shandilya wrote:
> Trackpad found in the Lenovo IdeaPad 330-15ARR U SKU
> 
> Signed-off-by: Harsh Shandilya <harsh@prjkt.io>
> Cc: stable@vger.kernel.org

Sorry, I noticed similar patch by Noah Westervelt first.

> ---
>  drivers/input/mouse/elan_i2c_core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index fb8806c1a39a..5a27944fb8de 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -1371,6 +1371,7 @@ static const struct acpi_device_id elan_acpi_id[] = {
>  	{ "ELAN0618", 0 },
>  	{ "ELAN061C", 0 },
>  	{ "ELAN061D", 0 },
> +	{ "ELAN061E", 0 },
>  	{ "ELAN0622", 0 },
>  	{ "ELAN1000", 0 },
>  	{ }
> -- 
> 2.20.0.rc0
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 1/1] Input: synaptics - enable SMBus for HP 15-ay000 (SYN3221).
From: Dmitry Torokhov @ 2018-12-03 19:26 UTC (permalink / raw)
  To: Teika Kazura; +Cc: linux-input, linux-kernel, Benjamin Tissoires
In-Reply-To: <20181116.130459.1796034910024118091.teika@gmx.com>

On Fri, Nov 16, 2018 at 01:04:59PM +0900, Teika Kazura wrote:
> SMBus works fine for the touchpad with id SYN3221, used in the HP 15-ay000 series,
> 
> This device has been reported in these messages in the "linux-input" mailing list:
> * https://marc.info/?l=linux-input&m=152016683003369&w=2
> * https://www.spinics.net/lists/linux-input/msg52525.html
> 
> Reported-by: Nitesh Debnath <niteshkd1999@gmail.com>
> Reported-by: Teika Kazura <teika@gmx.com>
> Signed-off-by: Teika Kazura <teika@gmx.com>

Applied, thank you.

> ---
>  drivers/input/mouse/synaptics.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 55d33500d5..591b776f22 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -179,6 +179,7 @@ static const char * const smbus_pnp_ids[] = {
>  	"LEN0096", /* X280 */
>  	"LEN0097", /* X280 -> ALPS trackpoint */
>  	"LEN200f", /* T450s */
> +	"SYN3221", /* HP 15-ay000 */
>  	NULL
>  };
> 
> --
> 2.18.1

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH 1/2] Input: omap-keypad: Fix keyboard debounce configuration
From: Dmitry Torokhov @ 2018-12-03 19:25 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: linux-input, linux-kernel, linux-omap, Axel Haslam, Felipe Balbi,
	Illia Smyrnov, Marcel Partap, Merlijn Wajer, Michael Scott, NeKit,
	Pavel Machek, Sebastian Reichel
In-Reply-To: <20181203012933.6647-1-tony@atomide.com>

On Sun, Dec 02, 2018 at 05:29:32PM -0800, Tony Lindgren wrote:
> I noticed that the Android v3.0.8 kernel on droid4 is using different
> keypad values from the mainline kernel and does not have issues with
> keys occasionally being stuck until pressed again. Turns out there was
> an earlier patch posted to fix this as "Input: omap-keypad: errata i689:
> Correct debounce time", but it was never reposted to fix use macros
> for timing calculations.
> 
> This updated version is using macros, and also fixes the use of the
> input clock rate to use 32768KiHz instead of 32000KiHz. And we want to
> use the known good Android kernel values of 3 and 6 instead of 2 and 6
> in the earlier patch.
> 
> Cc: Axel Haslam <axelhaslam@ti.com>
> Cc: Felipe Balbi <felipe.balbi@linux.intel.com>
> Cc: Illia Smyrnov <illia.smyrnov@ti.com>
> Cc: Marcel Partap <mpartap@gmx.net>
> Cc: Merlijn Wajer <merlijn@wizzup.org>
> Cc: Michael Scott <hashcode0f@gmail.com>
> Cc: NeKit <nekit1000@gmail.com>
> Cc: Pavel Machek <pavel@ucw.cz>
> Cc: Sebastian Reichel <sre@kernel.org>
> Reported-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Tony Lindgren <tony@atomide.com>

Applied, thank you.

> ---
>  drivers/input/keyboard/omap4-keypad.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
> --- a/drivers/input/keyboard/omap4-keypad.c
> +++ b/drivers/input/keyboard/omap4-keypad.c
> @@ -60,8 +60,18 @@
>  
>  /* OMAP4 values */
>  #define OMAP4_VAL_IRQDISABLE		0x0
> -#define OMAP4_VAL_DEBOUNCINGTIME	0x7
> -#define OMAP4_VAL_PVT			0x7
> +
> +/*
> + * Errata i689: If a key is released for a time shorter than debounce time,
> + * the keyboard will idle and never detect the key release. The workaround
> + * is to use at least a 12ms debounce time. See omap5432 TRM chapter
> + * "26.4.6.2 Keyboard Controller Timer" for more information.
> + */
> +#define OMAP4_KEYPAD_PTV_DIV_128        0x6
> +#define OMAP4_KEYPAD_DEBOUNCINGTIME_MS(dbms, ptv)     \
> +	((((dbms) * 1000) / ((1 << ((ptv) + 1)) * (1000000 / 32768))) - 1)
> +#define OMAP4_VAL_DEBOUNCINGTIME_16MS					\
> +	OMAP4_KEYPAD_DEBOUNCINGTIME_MS(16, OMAP4_KEYPAD_PTV_DIV_128)
>  
>  enum {
>  	KBD_REVISION_OMAP4 = 0,
> @@ -181,9 +191,9 @@ static int omap4_keypad_open(struct input_dev *input)
>  
>  	kbd_writel(keypad_data, OMAP4_KBD_CTRL,
>  			OMAP4_DEF_CTRL_NOSOFTMODE |
> -			(OMAP4_VAL_PVT << OMAP4_DEF_CTRL_PTV_SHIFT));
> +			(OMAP4_KEYPAD_PTV_DIV_128 << OMAP4_DEF_CTRL_PTV_SHIFT));
>  	kbd_writel(keypad_data, OMAP4_KBD_DEBOUNCINGTIME,
> -			OMAP4_VAL_DEBOUNCINGTIME);
> +			OMAP4_VAL_DEBOUNCINGTIME_16MS);
>  	/* clear pending interrupts */
>  	kbd_write_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS,
>  			 kbd_read_irqreg(keypad_data, OMAP4_KBD_IRQSTATUS));
> -- 
> 2.19.2

-- 
Dmitry

^ permalink raw reply


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