Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 5/7] HID: hid-steam: Update list of identifiers from SDL
From: Vicki Pfau @ 2023-12-20  3:38 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau
In-Reply-To: <20231220033837.2135194-1-vi@endrift.com>

SDL includes a list of settings (formerly called registers in this driver),
reports (formerly cmds), and various other identifiers that were provided by
Valve. This commit imports a significant chunk of that list as well as
replacing most of the guessed names and a handful of magic constants. It also
replaces bitmask definitions that used hex with the BIT macro.

Signed-off-by: Vicki Pfau <vi@endrift.com>
---
 drivers/hid/hid-steam.c | 286 +++++++++++++++++++++++++++++++---------
 1 file changed, 221 insertions(+), 65 deletions(-)

diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 667b5b09f931..4f5c647f04dd 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -71,51 +71,207 @@ static LIST_HEAD(steam_devices);
 
 /*
  * Commands that can be sent in a feature report.
- * Thanks to Valve for some valuable hints.
+ * Thanks to Valve and SDL for the names.
  */
-#define STEAM_CMD_SET_MAPPINGS		0x80
-#define STEAM_CMD_CLEAR_MAPPINGS	0x81
-#define STEAM_CMD_GET_MAPPINGS		0x82
-#define STEAM_CMD_GET_ATTRIB		0x83
-#define STEAM_CMD_GET_ATTRIB_LABEL	0x84
-#define STEAM_CMD_DEFAULT_MAPPINGS	0x85
-#define STEAM_CMD_FACTORY_RESET		0x86
-#define STEAM_CMD_WRITE_REGISTER	0x87
-#define STEAM_CMD_CLEAR_REGISTER	0x88
-#define STEAM_CMD_READ_REGISTER		0x89
-#define STEAM_CMD_GET_REGISTER_LABEL	0x8a
-#define STEAM_CMD_GET_REGISTER_MAX	0x8b
-#define STEAM_CMD_GET_REGISTER_DEFAULT	0x8c
-#define STEAM_CMD_SET_MODE		0x8d
-#define STEAM_CMD_DEFAULT_MOUSE		0x8e
-#define STEAM_CMD_FORCEFEEDBAK		0x8f
-#define STEAM_CMD_REQUEST_COMM_STATUS	0xb4
-#define STEAM_CMD_GET_SERIAL		0xae
-#define STEAM_CMD_HAPTIC_RUMBLE		0xeb
-
-/* Some useful register ids */
-#define STEAM_REG_LPAD_MODE		0x07
-#define STEAM_REG_RPAD_MODE		0x08
-#define STEAM_REG_RPAD_MARGIN		0x18
-#define STEAM_REG_LED			0x2d
-#define STEAM_REG_GYRO_MODE		0x30
-#define STEAM_REG_LPAD_CLICK_PRESSURE	0x34
-#define STEAM_REG_RPAD_CLICK_PRESSURE	0x35
-#define STEAM_REG_WATCHDOG_ENABLE		0x47
-
-/* Raw event identifiers */
-#define STEAM_EV_INPUT_DATA		0x01
-#define STEAM_EV_CONNECT		0x03
-#define STEAM_EV_BATTERY		0x04
-#define STEAM_EV_DECK_INPUT_DATA	0x09
+enum {
+	ID_SET_DIGITAL_MAPPINGS		= 0x80,
+	ID_CLEAR_DIGITAL_MAPPINGS	= 0x81,
+	ID_GET_DIGITAL_MAPPINGS		= 0x82,
+	ID_GET_ATTRIBUTES_VALUES	= 0x83,
+	ID_GET_ATTRIBUTE_LABEL		= 0x84,
+	ID_SET_DEFAULT_DIGITAL_MAPPINGS	= 0x85,
+	ID_FACTORY_RESET		= 0x86,
+	ID_SET_SETTINGS_VALUES		= 0x87,
+	ID_CLEAR_SETTINGS_VALUES	= 0x88,
+	ID_GET_SETTINGS_VALUES		= 0x89,
+	ID_GET_SETTING_LABEL		= 0x8A,
+	ID_GET_SETTINGS_MAXS		= 0x8B,
+	ID_GET_SETTINGS_DEFAULTS	= 0x8C,
+	ID_SET_CONTROLLER_MODE		= 0x8D,
+	ID_LOAD_DEFAULT_SETTINGS	= 0x8E,
+	ID_TRIGGER_HAPTIC_PULSE		= 0x8F,
+	ID_TURN_OFF_CONTROLLER		= 0x9F,
+
+	ID_GET_DEVICE_INFO		= 0xA1,
+
+	ID_CALIBRATE_TRACKPADS		= 0xA7,
+	ID_RESERVED_0			= 0xA8,
+	ID_SET_SERIAL_NUMBER		= 0xA9,
+	ID_GET_TRACKPAD_CALIBRATION	= 0xAA,
+	ID_GET_TRACKPAD_FACTORY_CALIBRATION = 0xAB,
+	ID_GET_TRACKPAD_RAW_DATA	= 0xAC,
+	ID_ENABLE_PAIRING		= 0xAD,
+	ID_GET_STRING_ATTRIBUTE		= 0xAE,
+	ID_RADIO_ERASE_RECORDS		= 0xAF,
+	ID_RADIO_WRITE_RECORD		= 0xB0,
+	ID_SET_DONGLE_SETTING		= 0xB1,
+	ID_DONGLE_DISCONNECT_DEVICE	= 0xB2,
+	ID_DONGLE_COMMIT_DEVICE		= 0xB3,
+	ID_DONGLE_GET_WIRELESS_STATE	= 0xB4,
+	ID_CALIBRATE_GYRO		= 0xB5,
+	ID_PLAY_AUDIO			= 0xB6,
+	ID_AUDIO_UPDATE_START		= 0xB7,
+	ID_AUDIO_UPDATE_DATA		= 0xB8,
+	ID_AUDIO_UPDATE_COMPLETE	= 0xB9,
+	ID_GET_CHIPID			= 0xBA,
+
+	ID_CALIBRATE_JOYSTICK		= 0xBF,
+	ID_CALIBRATE_ANALOG_TRIGGERS	= 0xC0,
+	ID_SET_AUDIO_MAPPING		= 0xC1,
+	ID_CHECK_GYRO_FW_LOAD		= 0xC2,
+	ID_CALIBRATE_ANALOG		= 0xC3,
+	ID_DONGLE_GET_CONNECTED_SLOTS	= 0xC4,
+
+	ID_RESET_IMU			= 0xCE,
+
+	ID_TRIGGER_HAPTIC_CMD		= 0xEA,
+	ID_TRIGGER_RUMBLE_CMD		= 0xEB,
+};
+
+/* Settings IDs */
+enum {
+	/* 0 */
+	SETTING_MOUSE_SENSITIVITY,
+	SETTING_MOUSE_ACCELERATION,
+	SETTING_TRACKBALL_ROTATION_ANGLE,
+	SETTING_HAPTIC_INTENSITY_UNUSED,
+	SETTING_LEFT_GAMEPAD_STICK_ENABLED,
+	SETTING_RIGHT_GAMEPAD_STICK_ENABLED,
+	SETTING_USB_DEBUG_MODE,
+	SETTING_LEFT_TRACKPAD_MODE,
+	SETTING_RIGHT_TRACKPAD_MODE,
+	SETTING_MOUSE_POINTER_ENABLED,
+
+	/* 10 */
+	SETTING_DPAD_DEADZONE,
+	SETTING_MINIMUM_MOMENTUM_VEL,
+	SETTING_MOMENTUM_DECAY_AMMOUNT,
+	SETTING_TRACKPAD_RELATIVE_MODE_TICKS_PER_PIXEL,
+	SETTING_HAPTIC_INCREMENT,
+	SETTING_DPAD_ANGLE_SIN,
+	SETTING_DPAD_ANGLE_COS,
+	SETTING_MOMENTUM_VERTICAL_DIVISOR,
+	SETTING_MOMENTUM_MAXIMUM_VELOCITY,
+	SETTING_TRACKPAD_Z_ON,
+
+	/* 20 */
+	SETTING_TRACKPAD_Z_OFF,
+	SETTING_SENSITIVY_SCALE_AMMOUNT,
+	SETTING_LEFT_TRACKPAD_SECONDARY_MODE,
+	SETTING_RIGHT_TRACKPAD_SECONDARY_MODE,
+	SETTING_SMOOTH_ABSOLUTE_MOUSE,
+	SETTING_STEAMBUTTON_POWEROFF_TIME,
+	SETTING_UNUSED_1,
+	SETTING_TRACKPAD_OUTER_RADIUS,
+	SETTING_TRACKPAD_Z_ON_LEFT,
+	SETTING_TRACKPAD_Z_OFF_LEFT,
+
+	/* 30 */
+	SETTING_TRACKPAD_OUTER_SPIN_VEL,
+	SETTING_TRACKPAD_OUTER_SPIN_RADIUS,
+	SETTING_TRACKPAD_OUTER_SPIN_HORIZONTAL_ONLY,
+	SETTING_TRACKPAD_RELATIVE_MODE_DEADZONE,
+	SETTING_TRACKPAD_RELATIVE_MODE_MAX_VEL,
+	SETTING_TRACKPAD_RELATIVE_MODE_INVERT_Y,
+	SETTING_TRACKPAD_DOUBLE_TAP_BEEP_ENABLED,
+	SETTING_TRACKPAD_DOUBLE_TAP_BEEP_PERIOD,
+	SETTING_TRACKPAD_DOUBLE_TAP_BEEP_COUNT,
+	SETTING_TRACKPAD_OUTER_RADIUS_RELEASE_ON_TRANSITION,
+
+	/* 40 */
+	SETTING_RADIAL_MODE_ANGLE,
+	SETTING_HAPTIC_INTENSITY_MOUSE_MODE,
+	SETTING_LEFT_DPAD_REQUIRES_CLICK,
+	SETTING_RIGHT_DPAD_REQUIRES_CLICK,
+	SETTING_LED_BASELINE_BRIGHTNESS,
+	SETTING_LED_USER_BRIGHTNESS,
+	SETTING_ENABLE_RAW_JOYSTICK,
+	SETTING_ENABLE_FAST_SCAN,
+	SETTING_IMU_MODE,
+	SETTING_WIRELESS_PACKET_VERSION,
+
+	/* 50 */
+	SETTING_SLEEP_INACTIVITY_TIMEOUT,
+	SETTING_TRACKPAD_NOISE_THRESHOLD,
+	SETTING_LEFT_TRACKPAD_CLICK_PRESSURE,
+	SETTING_RIGHT_TRACKPAD_CLICK_PRESSURE,
+	SETTING_LEFT_BUMPER_CLICK_PRESSURE,
+	SETTING_RIGHT_BUMPER_CLICK_PRESSURE,
+	SETTING_LEFT_GRIP_CLICK_PRESSURE,
+	SETTING_RIGHT_GRIP_CLICK_PRESSURE,
+	SETTING_LEFT_GRIP2_CLICK_PRESSURE,
+	SETTING_RIGHT_GRIP2_CLICK_PRESSURE,
+
+	/* 60 */
+	SETTING_PRESSURE_MODE,
+	SETTING_CONTROLLER_TEST_MODE,
+	SETTING_TRIGGER_MODE,
+	SETTING_TRACKPAD_Z_THRESHOLD,
+	SETTING_FRAME_RATE,
+	SETTING_TRACKPAD_FILT_CTRL,
+	SETTING_TRACKPAD_CLIP,
+	SETTING_DEBUG_OUTPUT_SELECT,
+	SETTING_TRIGGER_THRESHOLD_PERCENT,
+	SETTING_TRACKPAD_FREQUENCY_HOPPING,
+
+	/* 70 */
+	SETTING_HAPTICS_ENABLED,
+	SETTING_STEAM_WATCHDOG_ENABLE,
+	SETTING_TIMP_TOUCH_THRESHOLD_ON,
+	SETTING_TIMP_TOUCH_THRESHOLD_OFF,
+	SETTING_FREQ_HOPPING,
+	SETTING_TEST_CONTROL,
+	SETTING_HAPTIC_MASTER_GAIN_DB,
+	SETTING_THUMB_TOUCH_THRESH,
+	SETTING_DEVICE_POWER_STATUS,
+	SETTING_HAPTIC_INTENSITY,
+
+	/* 80 */
+	SETTING_STABILIZER_ENABLED,
+	SETTING_TIMP_MODE_MTE,
+};
+
+/* Input report identifiers */
+enum
+{
+	ID_CONTROLLER_STATE = 1,
+	ID_CONTROLLER_DEBUG = 2,
+	ID_CONTROLLER_WIRELESS = 3,
+	ID_CONTROLLER_STATUS = 4,
+	ID_CONTROLLER_DEBUG2 = 5,
+	ID_CONTROLLER_SECONDARY_STATE = 6,
+	ID_CONTROLLER_BLE_STATE = 7,
+	ID_CONTROLLER_DECK_STATE = 9
+};
+
+/* String attribute idenitifiers */
+enum {
+	ATTRIB_STR_BOARD_SERIAL,
+	ATTRIB_STR_UNIT_SERIAL,
+};
 
 /* Values for GYRO_MODE (bitmask) */
-#define STEAM_GYRO_MODE_OFF		0x0000
-#define STEAM_GYRO_MODE_STEERING	0x0001
-#define STEAM_GYRO_MODE_TILT		0x0002
-#define STEAM_GYRO_MODE_SEND_ORIENTATION	0x0004
-#define STEAM_GYRO_MODE_SEND_RAW_ACCEL		0x0008
-#define STEAM_GYRO_MODE_SEND_RAW_GYRO		0x0010
+enum {
+	SETTING_GYRO_MODE_OFF			= 0,
+	SETTING_GYRO_MODE_STEERING		= BIT(0),
+	SETTING_GYRO_MODE_TILT			= BIT(1),
+	SETTING_GYRO_MODE_SEND_ORIENTATION	= BIT(2),
+	SETTING_GYRO_MODE_SEND_RAW_ACCEL	= BIT(3),
+	SETTING_GYRO_MODE_SEND_RAW_GYRO		= BIT(4),
+};
+
+/* Trackpad modes */
+enum {
+	TRACKPAD_ABSOLUTE_MOUSE,
+	TRACKPAD_RELATIVE_MOUSE,
+	TRACKPAD_DPAD_FOUR_WAY_DISCRETE,
+	TRACKPAD_DPAD_FOUR_WAY_OVERLAP,
+	TRACKPAD_DPAD_EIGHT_WAY,
+	TRACKPAD_RADIAL_MODE,
+	TRACKPAD_ABSOLUTE_DPAD,
+	TRACKPAD_NONE,
+	TRACKPAD_GESTURE_KEYBOARD,
+};
 
 /* Other random constants */
 #define STEAM_SERIAL_LEN 10
@@ -226,13 +382,13 @@ static inline int steam_send_report_byte(struct steam_device *steam, u8 cmd)
 	return steam_send_report(steam, &cmd, 1);
 }
 
-static int steam_write_registers(struct steam_device *steam,
+static int steam_write_settings(struct steam_device *steam,
 		/* u8 reg, u16 val */...)
 {
 	/* Send: 0x87 len (reg valLo valHi)* */
 	u8 reg;
 	u16 val;
-	u8 cmd[64] = {STEAM_CMD_WRITE_REGISTER, 0x00};
+	u8 cmd[64] = {ID_SET_SETTINGS_VALUES, 0x00};
 	int ret;
 	va_list args;
 
@@ -268,7 +424,7 @@ static int steam_get_serial(struct steam_device *steam)
 	 * Recv: 0xae 0x15 0x01 serialnumber (10 chars)
 	 */
 	int ret = 0;
-	u8 cmd[] = {STEAM_CMD_GET_SERIAL, 0x15, 0x01};
+	u8 cmd[] = {ID_GET_STRING_ATTRIBUTE, 0x15, ATTRIB_STR_UNIT_SERIAL};
 	u8 reply[3 + STEAM_SERIAL_LEN + 1];
 
 	mutex_lock(&steam->report_mutex);
@@ -278,7 +434,7 @@ static int steam_get_serial(struct steam_device *steam)
 	ret = steam_recv_report(steam, reply, sizeof(reply));
 	if (ret < 0)
 		goto out;
-	if (reply[0] != 0xae || reply[1] != 0x15 || reply[2] != 0x01) {
+	if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] != 0x15 || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
 		ret = -EIO;
 		goto out;
 	}
@@ -298,7 +454,7 @@ static inline int steam_request_conn_status(struct steam_device *steam)
 {
 	int ret;
 	mutex_lock(&steam->report_mutex);
-	ret = steam_send_report_byte(steam, STEAM_CMD_REQUEST_COMM_STATUS);
+	ret = steam_send_report_byte(steam, ID_DONGLE_GET_WIRELESS_STATE);
 	mutex_unlock(&steam->report_mutex);
 	return ret;
 }
@@ -308,7 +464,7 @@ static inline int steam_haptic_rumble(struct steam_device *steam,
 				u8 left_gain, u8 right_gain)
 {
 	int ret;
-	u8 report[11] = {STEAM_CMD_HAPTIC_RUMBLE, 9};
+	u8 report[11] = {ID_TRIGGER_RUMBLE_CMD, 9};
 
 	report[3] = intensity & 0xFF;
 	report[4] = intensity >> 8;
@@ -351,28 +507,28 @@ static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
 	if (enable) {
 		mutex_lock(&steam->report_mutex);
 		/* enable esc, enter, cursors */
-		steam_send_report_byte(steam, STEAM_CMD_DEFAULT_MAPPINGS);
-		/* enable mouse */
-		steam_send_report_byte(steam, STEAM_CMD_DEFAULT_MOUSE);
+		steam_send_report_byte(steam, ID_SET_DEFAULT_DIGITAL_MAPPINGS);
+		/* reset settings */
+		steam_send_report_byte(steam, ID_LOAD_DEFAULT_SETTINGS);
 		mutex_unlock(&steam->report_mutex);
 	} else {
 		mutex_lock(&steam->report_mutex);
 		/* disable esc, enter, cursor */
-		steam_send_report_byte(steam, STEAM_CMD_CLEAR_MAPPINGS);
+		steam_send_report_byte(steam, ID_CLEAR_DIGITAL_MAPPINGS);
 
 		if (steam->quirks & STEAM_QUIRK_DECK) {
-			steam_write_registers(steam,
-				STEAM_REG_LPAD_MODE, 0x07, /* disable mouse */
-				STEAM_REG_RPAD_MODE, 0x07, /* disable mouse */
-				STEAM_REG_LPAD_CLICK_PRESSURE, 0xFFFF, /* disable clicky pad */
-				STEAM_REG_RPAD_CLICK_PRESSURE, 0xFFFF, /* disable clicky pad */
-				STEAM_REG_WATCHDOG_ENABLE, 0, /* disable watchdog that tests if Steam is active */
+			steam_write_settings(steam,
+				SETTING_LEFT_TRACKPAD_MODE, TRACKPAD_NONE, /* disable mouse */
+				SETTING_RIGHT_TRACKPAD_MODE, TRACKPAD_NONE, /* disable mouse */
+				SETTING_LEFT_TRACKPAD_CLICK_PRESSURE, 0xFFFF, /* disable haptic click */
+				SETTING_RIGHT_TRACKPAD_CLICK_PRESSURE, 0xFFFF, /* disable haptic click */
+				SETTING_STEAM_WATCHDOG_ENABLE, 0, /* disable watchdog that tests if Steam is active */
 				0);
 			mutex_unlock(&steam->report_mutex);
 		} else {
-			steam_write_registers(steam,
-				STEAM_REG_LPAD_MODE, 0x07, /* disable mouse */
-				STEAM_REG_RPAD_MODE, 0x07, /* disable mouse */
+			steam_write_settings(steam,
+				SETTING_LEFT_TRACKPAD_MODE, TRACKPAD_NONE, /* disable mouse */
+				SETTING_RIGHT_TRACKPAD_MODE, TRACKPAD_NONE, /* disable mouse */
 				0);
 			mutex_unlock(&steam->report_mutex);
 		}
@@ -1362,7 +1518,7 @@ static int steam_raw_event(struct hid_device *hdev,
 		return 0;
 
 	switch (data[2]) {
-	case STEAM_EV_INPUT_DATA:
+	case ID_CONTROLLER_STATE:
 		if (steam->client_opened)
 			return 0;
 		rcu_read_lock();
@@ -1371,7 +1527,7 @@ static int steam_raw_event(struct hid_device *hdev,
 			steam_do_input_event(steam, input, data);
 		rcu_read_unlock();
 		break;
-	case STEAM_EV_DECK_INPUT_DATA:
+	case ID_CONTROLLER_DECK_STATE:
 		if (steam->client_opened)
 			return 0;
 		rcu_read_lock();
@@ -1380,7 +1536,7 @@ static int steam_raw_event(struct hid_device *hdev,
 			steam_do_deck_input_event(steam, input, data);
 		rcu_read_unlock();
 		break;
-	case STEAM_EV_CONNECT:
+	case ID_CONTROLLER_WIRELESS:
 		/*
 		 * The payload of this event is a single byte:
 		 *  0x01: disconnected.
@@ -1395,7 +1551,7 @@ static int steam_raw_event(struct hid_device *hdev,
 			break;
 		}
 		break;
-	case STEAM_EV_BATTERY:
+	case ID_CONTROLLER_STATUS:
 		if (steam->quirks & STEAM_QUIRK_WIRELESS) {
 			rcu_read_lock();
 			battery = rcu_dereference(steam->battery);
-- 
2.42.0


^ permalink raw reply related

* [PATCH 7/7] HID: hid-steam: Add gamepad-only mode switched to by holding options
From: Vicki Pfau @ 2023-12-20  3:38 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau
In-Reply-To: <20231220033837.2135194-1-vi@endrift.com>

This commit adds a hotkey to switch between "gamepad" mode (mouse and keyboard
disabled) and "desktop" mode (gamepad disabled) by holding down the options
button (mapped here as the start button). This mirrors the behavior of the
official Steam client.

This also adds and uses a function for generating haptic pulses, as Steam also
does when engaging this hotkey.

Signed-off-by: Vicki Pfau <vi@endrift.com>
---
 drivers/hid/hid-steam.c | 113 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 103 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index a0ed8812e7ea..b3c4e50e248a 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -273,6 +273,11 @@ enum {
 	TRACKPAD_GESTURE_KEYBOARD,
 };
 
+/* Pad identifiers for the deck */
+#define STEAM_PAD_LEFT 0
+#define STEAM_PAD_RIGHT 1
+#define STEAM_PAD_BOTH 2
+
 /* Other random constants */
 #define STEAM_SERIAL_LEN 0x15
 
@@ -291,6 +296,9 @@ struct steam_device {
 	struct power_supply __rcu *battery;
 	u8 battery_charge;
 	u16 voltage;
+	struct delayed_work mode_switch;
+	bool did_mode_switch;
+	bool gamepad_mode;
 	struct work_struct rumble_work;
 	u16 rumble_left;
 	u16 rumble_right;
@@ -460,6 +468,37 @@ static inline int steam_request_conn_status(struct steam_device *steam)
 	return ret;
 }
 
+/*
+ * Send a haptic pulse to the trackpads
+ * Duration and interval are measured in microseconds, count is the number
+ * of pulses to send for duration time with interval microseconds between them
+ * and gain is measured in decibels, ranging from -24 to +6
+ */
+static inline int steam_haptic_pulse(struct steam_device *steam, u8 pad,
+				u16 duration, u16 interval, u16 count, u8 gain)
+{
+	int ret;
+	u8 report[10] = {ID_TRIGGER_HAPTIC_PULSE, 8};
+
+	/* Left and right are swapped on this report for legacy reasons */
+	if (pad < STEAM_PAD_BOTH)
+		pad ^= 1;
+
+	report[2] = pad;
+	report[3] = duration & 0xFF;
+	report[4] = duration >> 8;
+	report[5] = interval & 0xFF;
+	report[6] = interval >> 8;
+	report[7] = count & 0xFF;
+	report[8] = count >> 8;
+	report[9] = gain;
+
+	mutex_lock(&steam->report_mutex);
+	ret = steam_send_report(steam, report, sizeof(report));
+	mutex_unlock(&steam->report_mutex);
+	return ret;
+}
+
 static inline int steam_haptic_rumble(struct steam_device *steam,
 				u16 intensity, u16 left_speed, u16 right_speed,
 				u8 left_gain, u8 right_gain)
@@ -505,6 +544,9 @@ static int steam_play_effect(struct input_dev *dev, void *data,
 
 static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
 {
+	if (steam->gamepad_mode)
+		enable = false;
+
 	if (enable) {
 		mutex_lock(&steam->report_mutex);
 		/* enable esc, enter, cursors */
@@ -542,11 +584,18 @@ static int steam_input_open(struct input_dev *dev)
 	unsigned long flags;
 	bool set_lizard_mode;
 
-	spin_lock_irqsave(&steam->lock, flags);
-	set_lizard_mode = !steam->client_opened && lizard_mode;
-	spin_unlock_irqrestore(&steam->lock, flags);
-	if (set_lizard_mode)
-		steam_set_lizard_mode(steam, false);
+	/*
+	 * Disabling lizard mode automatically is only done on the Steam
+	 * Controller. On the Steam Deck, this is toggled manually by holding
+	 * the options button instead, handled by steam_mode_switch_cb.
+	 */
+	if (!(steam->quirks & STEAM_QUIRK_DECK)) {
+		spin_lock_irqsave(&steam->lock, flags);
+		set_lizard_mode = !steam->client_opened && lizard_mode;
+		spin_unlock_irqrestore(&steam->lock, flags);
+		if (set_lizard_mode)
+			steam_set_lizard_mode(steam, false);
+	}
 
 	return 0;
 }
@@ -557,11 +606,13 @@ static void steam_input_close(struct input_dev *dev)
 	unsigned long flags;
 	bool set_lizard_mode;
 
-	spin_lock_irqsave(&steam->lock, flags);
-	set_lizard_mode = !steam->client_opened && lizard_mode;
-	spin_unlock_irqrestore(&steam->lock, flags);
-	if (set_lizard_mode)
-		steam_set_lizard_mode(steam, true);
+	if (!(steam->quirks & STEAM_QUIRK_DECK)) {
+		spin_lock_irqsave(&steam->lock, flags);
+		set_lizard_mode = !steam->client_opened && lizard_mode;
+		spin_unlock_irqrestore(&steam->lock, flags);
+		if (set_lizard_mode)
+			steam_set_lizard_mode(steam, true);
+	}
 }
 
 static enum power_supply_property steam_battery_props[] = {
@@ -886,6 +937,34 @@ static void steam_work_connect_cb(struct work_struct *work)
 	}
 }
 
+static void steam_mode_switch_cb(struct work_struct *work)
+{
+	struct steam_device *steam = container_of(to_delayed_work(work),
+							struct steam_device, mode_switch);
+	unsigned long flags;
+	bool client_opened;
+	steam->gamepad_mode = !steam->gamepad_mode;
+	if (!lizard_mode)
+		return;
+
+	if (steam->gamepad_mode)
+		steam_set_lizard_mode(steam, false);
+	else {
+		spin_lock_irqsave(&steam->lock, flags);
+		client_opened = steam->client_opened;
+		spin_unlock_irqrestore(&steam->lock, flags);
+		if (!client_opened)
+			steam_set_lizard_mode(steam, lizard_mode);
+	}
+
+	steam_haptic_pulse(steam, STEAM_PAD_RIGHT, 0x190, 0, 1, 0);
+	if (steam->gamepad_mode) {
+		steam_haptic_pulse(steam, STEAM_PAD_LEFT, 0x14D, 0x14D, 0x2D, 0);
+	} else {
+		steam_haptic_pulse(steam, STEAM_PAD_LEFT, 0x1F4, 0x1F4, 0x1E, 0);
+	}
+}
+
 static bool steam_is_valve_interface(struct hid_device *hdev)
 {
 	struct hid_report_enum *rep_enum;
@@ -1040,6 +1119,7 @@ static int steam_probe(struct hid_device *hdev,
 	mutex_init(&steam->report_mutex);
 	steam->quirks = id->driver_data;
 	INIT_WORK(&steam->work_connect, steam_work_connect_cb);
+	INIT_DELAYED_WORK(&steam->mode_switch, steam_mode_switch_cb);
 	INIT_LIST_HEAD(&steam->list);
 	INIT_WORK(&steam->rumble_work, steam_haptic_rumble_cb);
 
@@ -1097,6 +1177,7 @@ static int steam_probe(struct hid_device *hdev,
 hid_hw_open_fail:
 hid_hw_start_fail:
 	cancel_work_sync(&steam->work_connect);
+	cancel_delayed_work_sync(&steam->mode_switch);
 	cancel_work_sync(&steam->rumble_work);
 steam_alloc_fail:
 	hid_err(hdev, "%s: failed with error %d\n",
@@ -1113,6 +1194,7 @@ static void steam_remove(struct hid_device *hdev)
 		return;
 	}
 
+	cancel_delayed_work_sync(&steam->mode_switch);
 	cancel_work_sync(&steam->work_connect);
 	hid_destroy_device(steam->client_hdev);
 	steam->client_hdev = NULL;
@@ -1398,6 +1480,17 @@ static void steam_do_deck_input_event(struct steam_device *steam,
 	b13 = data[13];
 	b14 = data[14];
 
+	if (!(b9 & BIT(6)) && steam->did_mode_switch) {
+		steam->did_mode_switch = false;
+		cancel_delayed_work_sync(&steam->mode_switch);
+	} else if (!steam->client_opened && (b9 & BIT(6)) && !steam->did_mode_switch) {
+		steam->did_mode_switch = true;
+		schedule_delayed_work(&steam->mode_switch, 45 * HZ / 100);
+	}
+
+	if (!steam->gamepad_mode)
+		return;
+
 	lpad_touched = b10 & BIT(3);
 	rpad_touched = b10 & BIT(4);
 
-- 
2.42.0


^ permalink raw reply related

* [PATCH 6/7] HID: hid-steam: Better handling of serial number length
From: Vicki Pfau @ 2023-12-20  3:38 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires, linux-input; +Cc: Vicki Pfau
In-Reply-To: <20231220033837.2135194-1-vi@endrift.com>

The second byte of the GET_STRING_ATTRIB report is a length, so we should set
the size of the buffer to be the size we're actually requesting, and only
reject the reply if the length out is nonsensical.

Signed-off-by: Vicki Pfau <vi@endrift.com>
---
 drivers/hid/hid-steam.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 4f5c647f04dd..a0ed8812e7ea 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -274,7 +274,7 @@ enum {
 };
 
 /* Other random constants */
-#define STEAM_SERIAL_LEN 10
+#define STEAM_SERIAL_LEN 0x15
 
 struct steam_device {
 	struct list_head list;
@@ -421,10 +421,10 @@ static int steam_get_serial(struct steam_device *steam)
 {
 	/*
 	 * Send: 0xae 0x15 0x01
-	 * Recv: 0xae 0x15 0x01 serialnumber (10 chars)
+	 * Recv: 0xae 0x15 0x01 serialnumber
 	 */
 	int ret = 0;
-	u8 cmd[] = {ID_GET_STRING_ATTRIBUTE, 0x15, ATTRIB_STR_UNIT_SERIAL};
+	u8 cmd[] = {ID_GET_STRING_ATTRIBUTE, sizeof(steam->serial_no), ATTRIB_STR_UNIT_SERIAL};
 	u8 reply[3 + STEAM_SERIAL_LEN + 1];
 
 	mutex_lock(&steam->report_mutex);
@@ -434,12 +434,13 @@ static int steam_get_serial(struct steam_device *steam)
 	ret = steam_recv_report(steam, reply, sizeof(reply));
 	if (ret < 0)
 		goto out;
-	if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] != 0x15 || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
+	if (reply[0] != ID_GET_STRING_ATTRIBUTE || reply[1] < 1 ||
+	    reply[1] > sizeof(steam->serial_no) || reply[2] != ATTRIB_STR_UNIT_SERIAL) {
 		ret = -EIO;
 		goto out;
 	}
 	reply[3 + STEAM_SERIAL_LEN] = 0;
-	strscpy(steam->serial_no, reply + 3, sizeof(steam->serial_no));
+	strscpy(steam->serial_no, reply + 3, reply[1]);
 out:
 	mutex_unlock(&steam->report_mutex);
 	return ret;
-- 
2.42.0


^ permalink raw reply related

* [PATCH 0/3] Add new SFH interfaces
From: Basavaraj Natikar @ 2023-12-20  7:00 UTC (permalink / raw)
  To: jikos, benjamin.tissoires, hdegoede, ilpo.jarvinen,
	mario.limonciello, Shyam-sundar.S-k, linux-input
  Cc: Basavaraj Natikar

This series adds new interfaces to export User presence information and
Ambient light to other drivers within the kernel.

Link: https://lore.kernel.org/all/ad064333-48a4-4cfa-9428-69e8a7c44667@redhat.com/

Basavaraj Natikar (3):
  HID: amd_sfh: rename float_to_int() to amd_sfh_float_to_int()
  HID: amd_sfh: Add a new interface for exporting HPD data
  HID: amd_sfh: Add a new interface for exporting ALS data

 drivers/hid/amd-sfh-hid/amd_sfh_common.h      |  6 ++
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 28 +++++----
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 20 +++++++
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 59 +++++++++++++++++++
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    |  2 +
 include/linux/amd-pmf-io.h                    | 50 ++++++++++++++++
 6 files changed, 152 insertions(+), 13 deletions(-)
 create mode 100644 include/linux/amd-pmf-io.h

-- 
2.25.1


^ permalink raw reply

* [PATCH 1/3] HID: amd_sfh: rename float_to_int() to amd_sfh_float_to_int()
From: Basavaraj Natikar @ 2023-12-20  7:00 UTC (permalink / raw)
  To: jikos, benjamin.tissoires, hdegoede, ilpo.jarvinen,
	mario.limonciello, Shyam-sundar.S-k, linux-input
  Cc: Basavaraj Natikar
In-Reply-To: <20231220070042.610455-1-Basavaraj.Natikar@amd.com>

Current amd_sfh driver has float_to_int() to convert units from
float to int. This is fine until this function gets called outside of
the current scope of file.

Add a prefix "amd_sfh" to float_to_int() so that function represents
the driver name. This function will be called by multiple callers in the
next patch.

Link: https://lore.kernel.org/all/ad064333-48a4-4cfa-9428-69e8a7c44667@redhat.com/
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c | 28 ++++++++++---------
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    |  1 +
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
index 8a037de08e92..33fbdde8aff0 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c
@@ -132,7 +132,7 @@ static void get_common_inputs(struct common_input_property *common, int report_i
 	common->event_type = HID_USAGE_SENSOR_EVENT_DATA_UPDATED_ENUM;
 }
 
-static int float_to_int(u32 flt32_val)
+int amd_sfh_float_to_int(u32 flt32_val)
 {
 	int fraction, shift, mantissa, sign, exp, zeropre;
 
@@ -201,9 +201,9 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
 			     OFFSET_SENSOR_DATA_DEFAULT;
 		memcpy_fromio(&accel_data, sensoraddr, sizeof(struct sfh_accel_data));
 		get_common_inputs(&acc_input.common_property, report_id);
-		acc_input.in_accel_x_value = float_to_int(accel_data.acceldata.x) / 100;
-		acc_input.in_accel_y_value = float_to_int(accel_data.acceldata.y) / 100;
-		acc_input.in_accel_z_value = float_to_int(accel_data.acceldata.z) / 100;
+		acc_input.in_accel_x_value = amd_sfh_float_to_int(accel_data.acceldata.x) / 100;
+		acc_input.in_accel_y_value = amd_sfh_float_to_int(accel_data.acceldata.y) / 100;
+		acc_input.in_accel_z_value = amd_sfh_float_to_int(accel_data.acceldata.z) / 100;
 		memcpy(input_report, &acc_input, sizeof(acc_input));
 		report_size = sizeof(acc_input);
 		break;
@@ -212,9 +212,9 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
 			     OFFSET_SENSOR_DATA_DEFAULT;
 		memcpy_fromio(&gyro_data, sensoraddr, sizeof(struct sfh_gyro_data));
 		get_common_inputs(&gyro_input.common_property, report_id);
-		gyro_input.in_angel_x_value = float_to_int(gyro_data.gyrodata.x) / 1000;
-		gyro_input.in_angel_y_value = float_to_int(gyro_data.gyrodata.y) / 1000;
-		gyro_input.in_angel_z_value = float_to_int(gyro_data.gyrodata.z) / 1000;
+		gyro_input.in_angel_x_value = amd_sfh_float_to_int(gyro_data.gyrodata.x) / 1000;
+		gyro_input.in_angel_y_value = amd_sfh_float_to_int(gyro_data.gyrodata.y) / 1000;
+		gyro_input.in_angel_z_value = amd_sfh_float_to_int(gyro_data.gyrodata.z) / 1000;
 		memcpy(input_report, &gyro_input, sizeof(gyro_input));
 		report_size = sizeof(gyro_input);
 		break;
@@ -223,9 +223,9 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
 			     OFFSET_SENSOR_DATA_DEFAULT;
 		memcpy_fromio(&mag_data, sensoraddr, sizeof(struct sfh_mag_data));
 		get_common_inputs(&magno_input.common_property, report_id);
-		magno_input.in_magno_x = float_to_int(mag_data.magdata.x) / 100;
-		magno_input.in_magno_y = float_to_int(mag_data.magdata.y) / 100;
-		magno_input.in_magno_z = float_to_int(mag_data.magdata.z) / 100;
+		magno_input.in_magno_x = amd_sfh_float_to_int(mag_data.magdata.x) / 100;
+		magno_input.in_magno_y = amd_sfh_float_to_int(mag_data.magdata.y) / 100;
+		magno_input.in_magno_z = amd_sfh_float_to_int(mag_data.magdata.z) / 100;
 		magno_input.in_magno_accuracy = mag_data.accuracy / 100;
 		memcpy(input_report, &magno_input, sizeof(magno_input));
 		report_size = sizeof(magno_input);
@@ -235,13 +235,15 @@ static u8 get_input_rep(u8 current_index, int sensor_idx, int report_id,
 			     OFFSET_SENSOR_DATA_DEFAULT;
 		memcpy_fromio(&als_data, sensoraddr, sizeof(struct sfh_als_data));
 		get_common_inputs(&als_input.common_property, report_id);
-		als_input.illuminance_value = float_to_int(als_data.lux);
+		als_input.illuminance_value = amd_sfh_float_to_int(als_data.lux);
 
 		memcpy_fromio(&binfo, mp2->vsbase, sizeof(struct sfh_base_info));
 		if (binfo.sbase.s_prop[ALS_IDX].sf.feat & 0x2) {
 			als_input.light_color_temp = als_data.light_color_temp;
-			als_input.chromaticity_x_value = float_to_int(als_data.chromaticity_x);
-			als_input.chromaticity_y_value = float_to_int(als_data.chromaticity_y);
+			als_input.chromaticity_x_value =
+				amd_sfh_float_to_int(als_data.chromaticity_x);
+			als_input.chromaticity_y_value =
+				amd_sfh_float_to_int(als_data.chromaticity_y);
 		}
 
 		report_size = sizeof(als_input);
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 656c3e95ef8c..75267b0fec70 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -166,4 +166,5 @@ struct hpd_status {
 
 void sfh_interface_init(struct amd_mp2_dev *mp2);
 void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
+int amd_sfh_float_to_int(u32 flt32_val);
 #endif
-- 
2.25.1


^ permalink raw reply related

* [PATCH 2/3] HID: amd_sfh: Add a new interface for exporting HPD data
From: Basavaraj Natikar @ 2023-12-20  7:00 UTC (permalink / raw)
  To: jikos, benjamin.tissoires, hdegoede, ilpo.jarvinen,
	mario.limonciello, Shyam-sundar.S-k, linux-input
  Cc: Basavaraj Natikar
In-Reply-To: <20231220070042.610455-1-Basavaraj.Natikar@amd.com>

AMDSFH has information about the User presence information via the Human
Presence Detection (HPD) sensor which is part of the AMD sensor fusion hub.

Add a new interface to export this information, where other drivers like
PMF can use this information to enhance user experiences.

Link: https://lore.kernel.org/all/ad064333-48a4-4cfa-9428-69e8a7c44667@redhat.com/
Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/hid/amd-sfh-hid/amd_sfh_common.h      |  5 ++
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c | 14 ++++++
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 37 +++++++++++++++
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.h    |  1 +
 include/linux/amd-pmf-io.h                    | 46 +++++++++++++++++++
 5 files changed, 103 insertions(+)
 create mode 100644 include/linux/amd-pmf-io.h

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index 2643bb14fee2..cd57037bf217 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -37,6 +37,10 @@ struct amd_mp2_sensor_info {
 	dma_addr_t dma_address;
 };
 
+struct sfh_dev_status {
+	bool is_hpd_present;
+};
+
 struct amd_mp2_dev {
 	struct pci_dev *pdev;
 	struct amdtp_cl_data *cl_data;
@@ -47,6 +51,7 @@ struct amd_mp2_dev {
 	struct amd_input_data in_data;
 	/* mp2 active control status */
 	u32 mp2_acs;
+	struct sfh_dev_status dev_en;
 };
 
 struct amd_mp2_ops {
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
index e9c6413af24a..0351b0fd394a 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
@@ -73,6 +73,12 @@ static int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
 	int i, status;
 
 	for (i = 0; i < cl_data->num_hid_devices; i++) {
+		switch (cl_data->sensor_idx[i]) {
+		case HPD_IDX:
+			privdata->dev_en.is_hpd_present = false;
+			break;
+		}
+
 		if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
 			privdata->mp2_ops->stop(privdata, cl_data->sensor_idx[i]);
 			status = amd_sfh_wait_for_response
@@ -178,6 +184,11 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
 			rc = amdtp_hid_probe(i, cl_data);
 			if (rc)
 				goto cleanup;
+			switch (cl_data->sensor_idx[i]) {
+			case HPD_IDX:
+				privdata->dev_en.is_hpd_present = true;
+				break;
+			}
 		}
 		dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
 			cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]),
@@ -259,6 +270,7 @@ static void amd_mp2_pci_remove(void *privdata)
 {
 	struct amd_mp2_dev *mp2 = privdata;
 
+	sfh_deinit_emp2();
 	amd_sfh_hid_client_deinit(privdata);
 	mp2->mp2_ops->stop_all(mp2);
 	pci_intx(mp2->pdev, false);
@@ -311,12 +323,14 @@ int amd_sfh1_1_init(struct amd_mp2_dev *mp2)
 
 	rc = amd_sfh_irq_init(mp2);
 	if (rc) {
+		sfh_deinit_emp2();
 		dev_err(dev, "amd_sfh_irq_init failed\n");
 		return rc;
 	}
 
 	rc = amd_sfh1_1_hid_client_init(mp2);
 	if (rc) {
+		sfh_deinit_emp2();
 		dev_err(dev, "amd_sfh1_1_hid_client_init failed\n");
 		return rc;
 	}
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 4f81ef2d4f56..197b828fc6a0 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -7,11 +7,14 @@
  *
  * Author: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
  */
+#include <linux/amd-pmf-io.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/iopoll.h>
 
 #include "amd_sfh_interface.h"
 
+static struct amd_mp2_dev *emp2;
+
 static int amd_sfh_wait_response(struct amd_mp2_dev *mp2, u8 sid, u32 cmd_id)
 {
 	struct sfh_cmd_response cmd_resp;
@@ -73,7 +76,41 @@ static struct amd_mp2_ops amd_sfh_ops = {
 	.response = amd_sfh_wait_response,
 };
 
+void sfh_deinit_emp2(void)
+{
+	emp2 = NULL;
+}
+
 void sfh_interface_init(struct amd_mp2_dev *mp2)
 {
 	mp2->mp2_ops = &amd_sfh_ops;
+	emp2 = mp2;
+}
+
+static int amd_sfh_hpd_info(u8 *user_present)
+{
+	struct hpd_status hpdstatus;
+
+	if (!user_present)
+		return -EINVAL;
+
+	if (!emp2 || !emp2->dev_en.is_hpd_present)
+		return -ENODEV;
+
+	hpdstatus.val = readl(emp2->mmio + AMD_C2P_MSG(4));
+	*user_present = hpdstatus.shpd.presence;
+
+	return 0;
+}
+
+int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
+{
+	if (sfh_info) {
+		switch (op) {
+		case MT_HPD:
+			return amd_sfh_hpd_info(&sfh_info->user_present);
+		}
+	}
+	return -EINVAL;
 }
+EXPORT_SYMBOL_GPL(amd_get_sfh_info);
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
index 75267b0fec70..2c211d28764d 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.h
@@ -165,6 +165,7 @@ struct hpd_status {
 };
 
 void sfh_interface_init(struct amd_mp2_dev *mp2);
+void sfh_deinit_emp2(void);
 void amd_sfh1_1_set_desc_ops(struct amd_mp2_ops *mp2_ops);
 int amd_sfh_float_to_int(u32 flt32_val);
 #endif
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
new file mode 100644
index 000000000000..5b6d29d36922
--- /dev/null
+++ b/include/linux/amd-pmf-io.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * AMD Platform Management Framework Interface
+ *
+ * Copyright (c) 2023, Advanced Micro Devices, Inc.
+ * All Rights Reserved.
+ *
+ * Authors: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
+ *          Basavaraj Natikar <Basavaraj.Natikar@amd.com>
+ */
+
+#ifndef AMD_PMF_IO_H
+#define AMD_PMF_IO_H
+
+#include <linux/types.h>
+
+/**
+ * enum sfh_message_type - Query the SFH message type
+ * @MT_HPD: Message ID to know the Human presence info from MP2 FW
+ */
+enum sfh_message_type {
+	MT_HPD,
+};
+
+/**
+ * enum sfh_hpd_info - Query the Human presence information
+ * @SFH_NOT_DETECTED: Check the HPD connection information from MP2 FW
+ * @SFH_USER_PRESENT: Check if the user is present from HPD sensor
+ * @SFH_USER_AWAY: Check if the user is away from HPD sensor
+ */
+enum sfh_hpd_info {
+	SFH_NOT_DETECTED,
+	SFH_USER_PRESENT,
+	SFH_USER_AWAY,
+};
+
+/**
+ * struct amd_sfh_info - get HPD sensor info from MP2 FW
+ * @user_present: Populates the user presence information
+ */
+struct amd_sfh_info {
+	u8 user_present;
+};
+
+int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op);
+#endif
-- 
2.25.1


^ permalink raw reply related

* [PATCH 3/3] HID: amd_sfh: Add a new interface for exporting ALS data
From: Basavaraj Natikar @ 2023-12-20  7:00 UTC (permalink / raw)
  To: jikos, benjamin.tissoires, hdegoede, ilpo.jarvinen,
	mario.limonciello, Shyam-sundar.S-k, linux-input
  Cc: Basavaraj Natikar
In-Reply-To: <20231220070042.610455-1-Basavaraj.Natikar@amd.com>

AMDSFH has information about the Ambient light via the Ambient
Light Sensor (ALS) which is part of the AMD sensor fusion hub.

Add a new interface to export this information, where other drivers like
PMF can use this information to enhance user experiences.

Link: https://lore.kernel.org/all/ad064333-48a4-4cfa-9428-69e8a7c44667@redhat.com/
Reviewed-by: Mario Limonciello <mario.limonciello@amd.com>
Co-developed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
---
 drivers/hid/amd-sfh-hid/amd_sfh_common.h      |  1 +
 drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c |  6 +++++
 .../amd-sfh-hid/sfh1_1/amd_sfh_interface.c    | 22 +++++++++++++++++++
 include/linux/amd-pmf-io.h                    |  4 ++++
 4 files changed, 33 insertions(+)

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_common.h b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
index cd57037bf217..a1950bc6e6ce 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_common.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_common.h
@@ -39,6 +39,7 @@ struct amd_mp2_sensor_info {
 
 struct sfh_dev_status {
 	bool is_hpd_present;
+	bool is_als_present;
 };
 
 struct amd_mp2_dev {
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
index 0351b0fd394a..9dbe6f4cb294 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_init.c
@@ -77,6 +77,9 @@ static int amd_sfh_hid_client_deinit(struct amd_mp2_dev *privdata)
 		case HPD_IDX:
 			privdata->dev_en.is_hpd_present = false;
 			break;
+		case ALS_IDX:
+			privdata->dev_en.is_als_present = false;
+			break;
 		}
 
 		if (cl_data->sensor_sts[i] == SENSOR_ENABLED) {
@@ -188,6 +191,9 @@ static int amd_sfh1_1_hid_client_init(struct amd_mp2_dev *privdata)
 			case HPD_IDX:
 				privdata->dev_en.is_hpd_present = true;
 				break;
+			case ALS_IDX:
+				privdata->dev_en.is_als_present = true;
+				break;
 			}
 		}
 		dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
diff --git a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
index 197b828fc6a0..ae36312bc236 100644
--- a/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
+++ b/drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_interface.c
@@ -103,12 +103,34 @@ static int amd_sfh_hpd_info(u8 *user_present)
 	return 0;
 }
 
+static int amd_sfh_als_info(u32 *ambient_light)
+{
+	struct sfh_als_data als_data;
+	void __iomem *sensoraddr;
+
+	if (!ambient_light)
+		return -EINVAL;
+
+	if (!emp2 || !emp2->dev_en.is_als_present)
+		return -ENODEV;
+
+	sensoraddr = emp2->vsbase +
+		(ALS_IDX * SENSOR_DATA_MEM_SIZE_DEFAULT) +
+		OFFSET_SENSOR_DATA_DEFAULT;
+	memcpy_fromio(&als_data, sensoraddr, sizeof(struct sfh_als_data));
+	*ambient_light = amd_sfh_float_to_int(als_data.lux);
+
+	return 0;
+}
+
 int amd_get_sfh_info(struct amd_sfh_info *sfh_info, enum sfh_message_type op)
 {
 	if (sfh_info) {
 		switch (op) {
 		case MT_HPD:
 			return amd_sfh_hpd_info(&sfh_info->user_present);
+		case MT_ALS:
+			return amd_sfh_als_info(&sfh_info->ambient_light);
 		}
 	}
 	return -EINVAL;
diff --git a/include/linux/amd-pmf-io.h b/include/linux/amd-pmf-io.h
index 5b6d29d36922..b4f818205216 100644
--- a/include/linux/amd-pmf-io.h
+++ b/include/linux/amd-pmf-io.h
@@ -17,9 +17,11 @@
 /**
  * enum sfh_message_type - Query the SFH message type
  * @MT_HPD: Message ID to know the Human presence info from MP2 FW
+ * @MT_ALS: Message ID to know the Ambient light info from MP2 FW
  */
 enum sfh_message_type {
 	MT_HPD,
+	MT_ALS,
 };
 
 /**
@@ -36,9 +38,11 @@ enum sfh_hpd_info {
 
 /**
  * struct amd_sfh_info - get HPD sensor info from MP2 FW
+ * @ambient_light: Populates the ambient light information
  * @user_present: Populates the user presence information
  */
 struct amd_sfh_info {
+	u32 ambient_light;
 	u8 user_present;
 };
 
-- 
2.25.1


^ permalink raw reply related

* [PATCH 2/3] HID: make ishtp_cl_bus_type const
From: Greg Kroah-Hartman @ 2023-12-20  7:38 UTC (permalink / raw)
  To: jikos, benjamin.tissoires
  Cc: linux-kernel, Greg Kroah-Hartman, Srinivas Pandruvada,
	linux-input
In-Reply-To: <2023122045-pellet-eggbeater-8d2f@gregkh>

Now that the driver core can properly handle constant struct bus_type,
move the ishtp_cl_bus_type variable to be a constant structure as well,
placing it into read-only memory which can not be modified at runtime.

Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/hid/intel-ish-hid/ishtp/bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c b/drivers/hid/intel-ish-hid/ishtp/bus.c
index 7fc738a22375..aa6cb033bb06 100644
--- a/drivers/hid/intel-ish-hid/ishtp/bus.c
+++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
@@ -378,7 +378,7 @@ static const struct dev_pm_ops ishtp_cl_bus_dev_pm_ops = {
 	.restore = ishtp_cl_device_resume,
 };
 
-static struct bus_type ishtp_cl_bus_type = {
+static const struct bus_type ishtp_cl_bus_type = {
 	.name		= "ishtp",
 	.dev_groups	= ishtp_cl_dev_groups,
 	.probe		= ishtp_cl_device_probe,
-- 
2.43.0


^ permalink raw reply related

* [PATCH 1/3] HID: make hid_bus_type const
From: Greg Kroah-Hartman @ 2023-12-20  7:38 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-kernel, Greg Kroah-Hartman, linux-input

Now that the driver core can properly handle constant struct bus_type,
move the hid_bus_type variable to be a constant structure as well,
placing it into read-only memory which can not be modified at runtime.

Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/hid/hid-core.c | 2 +-
 include/linux/hid.h    | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index e0181218ad85..de7a477d6665 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2749,7 +2749,7 @@ static int hid_uevent(const struct device *dev, struct kobj_uevent_env *env)
 	return 0;
 }
 
-struct bus_type hid_bus_type = {
+const struct bus_type hid_bus_type = {
 	.name		= "hid",
 	.dev_groups	= hid_dev_groups,
 	.drv_groups	= hid_drv_groups,
diff --git a/include/linux/hid.h b/include/linux/hid.h
index bf43f3ff6664..7c26db874ff0 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -912,7 +912,7 @@ extern bool hid_ignore(struct hid_device *);
 extern int hid_add_device(struct hid_device *);
 extern void hid_destroy_device(struct hid_device *);
 
-extern struct bus_type hid_bus_type;
+extern const struct bus_type hid_bus_type;
 
 extern int __must_check __hid_register_driver(struct hid_driver *,
 		struct module *, const char *mod_name);
-- 
2.43.0


^ permalink raw reply related

* [PATCH 3/3] HID: bpf: make bus_type const in struct hid_bpf_ops
From: Greg Kroah-Hartman @ 2023-12-20  7:38 UTC (permalink / raw)
  To: jikos, benjamin.tissoires; +Cc: linux-kernel, Greg Kroah-Hartman, linux-input
In-Reply-To: <2023122045-pellet-eggbeater-8d2f@gregkh>

The struct bus_type pointer in hid_bpf_ops just passes the pointer to
the driver core, and the driver core can handle, and expects, a constant
pointer, so also make the pointer constant in hid_bpf_ops.

Part of the process of moving all usages of struct bus_type to be
constant to move them all to read-only memory.

Cc: Jiri Kosina <jikos@kernel.org>
Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Cc: linux-input@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 include/linux/hid_bpf.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h
index e9afb61e6ee0..840cd254172d 100644
--- a/include/linux/hid_bpf.h
+++ b/include/linux/hid_bpf.h
@@ -115,7 +115,7 @@ struct hid_bpf_ops {
 				  size_t len, enum hid_report_type rtype,
 				  enum hid_class_request reqtype);
 	struct module *owner;
-	struct bus_type *bus_type;
+	const struct bus_type *bus_type;
 };
 
 extern struct hid_bpf_ops *hid_bpf_ops;
-- 
2.43.0


^ permalink raw reply related

* [PATCH v6 0/4] Input: support overlay objects on touchscreens
From: Javier Carrasco @ 2023-12-20  8:39 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, Bastian Hecht, Michael Riesch
  Cc: linux-kernel, linux-input, devicetree, Javier Carrasco

Some touchscreens are shipped with a physical layer on top of them where
a number of buttons and a resized touchscreen surface might be available.

In order to generate proper key events by overlay buttons and adjust the
touch events to a clipped surface, this series offers a documented,
device-tree-based solution by means of helper functions.
An implementation for a specific touchscreen driver is also included.

The functions in touch-overlay provide a simple workflow to acquire
physical objects from the device tree, map them into a list and generate
events according to the object descriptions.

This feature has been tested with a JT240MHQS-E3 display, which consists
of an st1624 as the base touchscreen and an overlay with two buttons and
a frame that clips its effective surface mounted on it.

Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
Changes in v6:
- General: use a single list to manage a single type of object.
- General: swap patches to have bindings preceding the code.
- touch-overlay.c: minor code-sytle fixes.
- Link to v5: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v5-0-ff6b5c4db693@wolfvision.net

Changes in v5:
- touchscreen bindings: move overlay common properties to a $def entry (Rob Herring)
- st1232 bindings: move overlays to the existing example instead of
  making a new one (Rob Herring) 
- Link to v4: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v4-0-5c6c0fc1eed6@wolfvision.net

Changes in v4:
- General: rename "touchscreen" to "touch" to include other consumers.
- PATCH 1/4: move touch-overlay feature to input core.
- PATCH 1/4, 3/4: set key caps and report key events without consumer's
  intervention.
- PATCH 2/4: add missing 'required' field with the required properties.
- Link to v3: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v3-0-b4fb7fc4bab7@wolfvision.net

Changes in v3:
- General: rename "virtobj" and "virtual" to "overlay"
- PATCH 1/4: Make feature bool instead of tristate (selected by
  supported touchscreens)
- Link to v2: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v2-0-f68a6bfe7a0f@wolfvision.net

Changes in v2:
- PATCH 1/4: remove preprocessor directives (the module is selected by
  the drivers that support the feature). Typo in the commit message.
- PATCH 2/4: more detailed documentation. Images and examples were added.
- PATCH 3/4: select ts-virtobj automatically.
- Link to v1: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v1-0-5ae5e81bc264@wolfvision.net

---
Javier Carrasco (4):
      dt-bindings: touchscreen: add touch-overlay property
      Input: touch-overlay - Add touchscreen overlay handling
      dt-bindings: input: touchscreen: st1232: add touch-overlay example
      Input: st1232 - add touch overlays handling

 .../input/touchscreen/sitronix,st1232.yaml         |  29 +++
 .../bindings/input/touchscreen/touchscreen.yaml    | 119 +++++++++
 MAINTAINERS                                        |   7 +
 drivers/input/Makefile                             |   2 +-
 drivers/input/touch-overlay.c                      | 283 +++++++++++++++++++++
 drivers/input/touchscreen/st1232.c                 |  71 +++++-
 include/linux/input/touch-overlay.h                |  24 ++
 7 files changed, 520 insertions(+), 15 deletions(-)
---
base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
change-id: 20230510-feature-ts_virtobj_patch-e267540aae74

Best regards,
-- 
Javier Carrasco <javier.carrasco@wolfvision.net>


^ permalink raw reply

* [PATCH v6 1/4] dt-bindings: touchscreen: add touch-overlay property
From: Javier Carrasco @ 2023-12-20  8:39 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, Bastian Hecht, Michael Riesch
  Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v6-0-d8a605975153@wolfvision.net>

The touch-overlay encompasses a number of touch areas that define a
clipped touchscreen area and/or buttons with a specific functionality.

A clipped touchscreen area avoids getting events from regions that are
physically hidden by overlay frames.

For touchscreens with printed overlay buttons, sub-nodes with a suitable
key code can be defined to report key events instead of the original
touch events.

Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
 .../bindings/input/touchscreen/touchscreen.yaml    | 119 +++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
index 431c13335c40..d5ac90667bef 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/touchscreen.yaml
@@ -87,6 +87,125 @@ properties:
   touchscreen-y-plate-ohms:
     description: Resistance of the Y-plate in Ohms
 
+  touch-overlay:
+    description: list of nodes defining segments (touch areas) on the
+      touchscreen.
+
+      This object can be used to describe a series of segments to
+      restrict the region within touch events are reported or buttons
+      with a specific functionality.
+
+      This is of special interest if the touchscreen is shipped with a physical
+      overlay on top of it with a frame that hides some part of the original
+      touchscreen area. Printed buttons on that overlay are also a typical
+      use case.
+
+      A new touchscreen area is defined as a sub-node without a key code. If a
+      key code is defined in the sub-node, it will be interpreted as a button.
+
+      The x-origin and y-origin properties of a touchscreen area define the
+      offset of a new origin from where the touchscreen events are referenced.
+      This offset is applied to the events accordingly. The x-size and y-size
+      properties define the size of the touchscreen effective area.
+
+      The following example shows a new touchscreen area with the new origin
+      (0',0') for the touch events generated by the device.
+
+                   Touchscreen (full area)
+         ┌────────────────────────────────────────┐
+         │    ┌───────────────────────────────┐   │
+         │    │                               │   │
+         │    ├ y-size                        │   │
+         │    │                               │   │
+         │    │       touchscreen area        │   │
+         │    │         (no key code)         │   │
+         │    │                               │   │
+         │    │            x-size             │   │
+         │   ┌└──────────────┴────────────────┘   │
+         │(0',0')                                 │
+        ┌└────────────────────────────────────────┘
+      (0,0)
+
+     where (0',0') = (0+x-origin,0+y-origin)
+
+      Sub-nodes with key codes report the touch events on their surface as key
+      events instead.
+
+      The following example shows a touchscreen with a single button on it.
+
+              Touchscreen (full area)
+        ┌───────────────────────────────────┐
+        │                                   │
+        │                                   │
+        │   ┌─────────┐                     │
+        │   │button 0 │                     │
+        │   │KEY_POWER│                     │
+        │   └─────────┘                     │
+        │                                   │
+        │                                   │
+       ┌└───────────────────────────────────┘
+     (0,0)
+
+      Segments defining buttons and clipped toushcreen areas can be combined
+      as shown in the following example.
+      In that case only the events within the touchscreen area are reported
+      as touch events. Events within the button areas report their associated
+      key code. Any events outside the defined areas are ignored.
+
+                  Touchscreen (full area)
+        ┌─────────┬──────────────────────────────┐
+        │         │                              │
+        │         │    ┌───────────────────────┐ │
+        │ button 0│    │                       │ │
+        │KEY_POWER│    │                       │ │
+        │         │    │                       │ │
+        ├─────────┤    │   touchscreen area    │ │
+        │         │    │     (no key code)     │ │
+        │         │    │                       │ │
+        │ button 1│    │                       │ │
+        │ KEY_INFO│   ┌└───────────────────────┘ │
+        │         │(0',0')                       │
+       ┌└─────────┴──────────────────────────────┘
+     (0,0)
+
+    type: object
+
+    patternProperties:
+      '^segment-':
+        type: object
+        description:
+          Each segment is represented as a sub-node.
+        properties:
+          x-origin:
+            description: horizontal origin of the node area
+            $ref: /schemas/types.yaml#/definitions/uint32
+
+          y-origin:
+            description: vertical origin of the node area
+            $ref: /schemas/types.yaml#/definitions/uint32
+
+          x-size:
+            description: horizontal resolution of the node area
+            $ref: /schemas/types.yaml#/definitions/uint32
+
+          y-size:
+            description: vertical resolution of the node area
+            $ref: /schemas/types.yaml#/definitions/uint32
+
+          label:
+            $ref: /schemas/types.yaml#/definitions/string
+            description: descriptive name of the button
+
+          linux,code: true
+
+        required:
+          - x-origin
+          - y-origin
+          - x-size
+          - y-size
+
+        unevaluatedProperties: false
+
 dependencies:
   touchscreen-size-x: [ touchscreen-size-y ]
   touchscreen-size-y: [ touchscreen-size-x ]

-- 
2.39.2


^ permalink raw reply related

* [PATCH v6 2/4] Input: touch-overlay - Add touchscreen overlay handling
From: Javier Carrasco @ 2023-12-20  8:39 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, Bastian Hecht, Michael Riesch
  Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v6-0-d8a605975153@wolfvision.net>

Some touch devices provide mechanical overlays with different objects
like buttons or clipped touchscreen surfaces.

In order to support these objects, add a series of helper functions
to the input subsystem to transform them into overlay objects via
device tree nodes.

These overlay objects consume the raw touch events and report the
expected input events depending on the object properties.

Note that the current implementation allows for multiple definitions
of touchscreen areas (regions that report touch events), but only the
first one will be used for the touchscreen device that the consumers
typically provide.
Should the need for multiple touchscreen areas arise, additional
touchscreen devices would be required at the consumer side.
There is no limitation in the number of touch areas defined as buttons.

Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
 MAINTAINERS                         |   7 +
 drivers/input/Makefile              |   2 +-
 drivers/input/touch-overlay.c       | 283 ++++++++++++++++++++++++++++++++++++
 include/linux/input/touch-overlay.h |  24 +++
 4 files changed, 315 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 97f51d5ec1cf..3218d8694735 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22031,6 +22031,13 @@ L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
 F:	drivers/platform/x86/toshiba-wmi.c
 
+TOUCH OVERLAY
+M:	Javier Carrasco <javier.carrasco@wolfvision.net>
+L:	linux-input@vger.kernel.org
+S:	Maintained
+F:	drivers/input/touch-overlay.c
+F:	include/linux/input/touch-overlay.h
+
 TPM DEVICE DRIVER
 M:	Peter Huewe <peterhuewe@gmx.de>
 M:	Jarkko Sakkinen <jarkko@kernel.org>
diff --git a/drivers/input/Makefile b/drivers/input/Makefile
index c78753274921..393e9f4d00dc 100644
--- a/drivers/input/Makefile
+++ b/drivers/input/Makefile
@@ -7,7 +7,7 @@
 
 obj-$(CONFIG_INPUT)		+= input-core.o
 input-core-y := input.o input-compat.o input-mt.o input-poller.o ff-core.o
-input-core-y += touchscreen.o
+input-core-y += touchscreen.o touch-overlay.o
 
 obj-$(CONFIG_INPUT_FF_MEMLESS)	+= ff-memless.o
 obj-$(CONFIG_INPUT_SPARSEKMAP)	+= sparse-keymap.o
diff --git a/drivers/input/touch-overlay.c b/drivers/input/touch-overlay.c
new file mode 100644
index 000000000000..0a0646ceb755
--- /dev/null
+++ b/drivers/input/touch-overlay.c
@@ -0,0 +1,283 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ *  Helper functions for overlay objects on touchscreens
+ *
+ *  Copyright (c) 2023 Javier Carrasco <javier.carrasco@wolfvision.net>
+ */
+
+#include <linux/input.h>
+#include <linux/input/mt.h>
+#include <linux/input/touch-overlay.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/property.h>
+
+struct button {
+	u32 key;
+	bool pressed;
+	int slot;
+};
+
+struct touch_overlay_segment {
+	struct list_head list;
+	u32 x_origin;
+	u32 y_origin;
+	u32 x_size;
+	u32 y_size;
+	struct button *btn;
+};
+
+static int touch_overlay_get_segment(struct fwnode_handle *segment_node,
+				     struct touch_overlay_segment *segment,
+				     struct input_dev *keypad,
+				     struct device *dev)
+{
+	int error;
+
+	error = fwnode_property_read_u32(segment_node, "x-origin",
+					 &segment->x_origin);
+	if (error)
+		return error;
+
+	error = fwnode_property_read_u32(segment_node, "y-origin",
+					 &segment->y_origin);
+	if (error)
+		return error;
+
+	error = fwnode_property_read_u32(segment_node, "x-size",
+					 &segment->x_size);
+	if (error)
+		return error;
+
+	error = fwnode_property_read_u32(segment_node, "y-size",
+					 &segment->y_size);
+	if (error)
+		return error;
+
+	if (fwnode_property_present(segment_node, "linux,code")) {
+		segment->btn = devm_kzalloc(dev, sizeof(*segment->btn),
+					    GFP_KERNEL);
+		if (!segment->btn)
+			return -ENOMEM;
+
+		error = fwnode_property_read_u32(segment_node, "linux,code",
+						 &segment->btn->key);
+		if (error)
+			return error;
+
+		input_set_capability(keypad, EV_KEY, segment->btn->key);
+	}
+
+	return 0;
+}
+
+/**
+ * touch_overlay_map - map overlay objects from the device tree and set
+ * key capabilities if buttons are defined.
+ * @list: pointer to the list that will hold the segments
+ * @keypad: pointer to the already allocated input_dev
+ *
+ * Returns 0 on success and error number otherwise.
+ *
+ * If a keypad input device is provided and overlay buttons are defined,
+ * its button capabilities are set accordingly.
+ */
+int touch_overlay_map(struct list_head *list, struct input_dev *keypad)
+{
+	struct fwnode_handle *overlay, *fw_segment;
+	struct device *dev = keypad->dev.parent;
+	struct touch_overlay_segment *segment;
+	int error;
+
+	overlay = device_get_named_child_node(dev, "touch-overlay");
+	if (!overlay)
+		return 0;
+
+	fwnode_for_each_child_node(overlay, fw_segment) {
+		segment = devm_kzalloc(dev, sizeof(*segment), GFP_KERNEL);
+		if (!segment) {
+			error = -ENOMEM;
+			goto put_overlay;
+		}
+		error = touch_overlay_get_segment(fw_segment, segment, keypad,
+						  dev);
+		if (error)
+			goto put_overlay;
+
+		list_add_tail(&segment->list, list);
+	}
+
+put_overlay:
+	fwnode_handle_put(overlay);
+	return error;
+}
+EXPORT_SYMBOL(touch_overlay_map);
+
+/**
+ * touch_overlay_get_touchscreen_abs - get abs size from the touchscreen area.
+ * @list: pointer to the list that holds the segments
+ * @x: horizontal abs
+ * @y: vertical abs
+ */
+void touch_overlay_get_touchscreen_abs(struct list_head *list, u16 *x, u16 *y)
+{
+	struct touch_overlay_segment *segment;
+	struct list_head *ptr;
+
+	list_for_each(ptr, list) {
+		segment = list_entry(ptr, struct touch_overlay_segment, list);
+		if (!segment->btn) {
+			*x = segment->x_size - 1;
+			*y = segment->y_size - 1;
+			break;
+		}
+	}
+}
+EXPORT_SYMBOL(touch_overlay_get_touchscreen_abs);
+
+static bool touch_overlay_segment_event(struct touch_overlay_segment *seg,
+					u32 x, u32 y)
+{
+	if (!seg)
+		return false;
+
+	if (x >= seg->x_origin && x < (seg->x_origin + seg->x_size) &&
+	    y >= seg->y_origin && y < (seg->y_origin + seg->y_size))
+		return true;
+
+	return false;
+}
+
+/**
+ * touch_overlay_mapped_touchscreen - check if a touchscreen area is mapped
+ * @list: pointer to the list that holds the segments
+ *
+ * Returns true if a touchscreen area is mapped or false otherwise.
+ */
+bool touch_overlay_mapped_touchscreen(struct list_head *list)
+{
+	struct touch_overlay_segment *segment;
+	struct list_head *ptr;
+
+	list_for_each(ptr, list) {
+		segment = list_entry(ptr, struct touch_overlay_segment, list);
+		if (!segment->btn)
+			return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(touch_overlay_mapped_touchscreen);
+
+/**
+ * touch_overlay_mapped_buttons - check if overlay buttons are mapped
+ * @list: pointer to the list that holds the segments
+ *
+ * Returns true if overlay buttons mapped or false otherwise.
+ */
+bool touch_overlay_mapped_buttons(struct list_head *list)
+{
+	struct touch_overlay_segment *segment;
+	struct list_head *ptr;
+
+	list_for_each(ptr, list) {
+		segment = list_entry(ptr, struct touch_overlay_segment, list);
+		if (segment->btn)
+			return true;
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(touch_overlay_mapped_buttons);
+
+static bool touch_overlay_mt_on_touchscreen(struct list_head *list,
+					    u32 *x, u32 *y)
+{
+	struct touch_overlay_segment *segment;
+	bool contact = x && y;
+	struct list_head *ptr;
+
+	/* Let the caller handle events with no coordinates (release) */
+	if (!contact)
+		return false;
+
+	list_for_each(ptr, list) {
+		segment = list_entry(ptr, struct touch_overlay_segment, list);
+		if (!segment->btn &&
+		    touch_overlay_segment_event(segment, *x, *y)) {
+			*x -= segment->x_origin;
+			*y -= segment->y_origin;
+			return true;
+		}
+	}
+
+	return false;
+}
+
+static bool touch_overlay_button_event(struct input_dev *input,
+				       struct touch_overlay_segment *segment,
+				       const u32 *x, const u32 *y, u32 slot)
+{
+	bool contact = x && y;
+
+	if (!contact && segment->btn->pressed && segment->btn->slot == slot) {
+		segment->btn->pressed = false;
+		input_report_key(input, segment->btn->key, false);
+		input_sync(input);
+		return true;
+	} else if (contact && touch_overlay_segment_event(segment, *x, *y)) {
+		segment->btn->pressed = true;
+		segment->btn->slot = slot;
+		input_report_key(input, segment->btn->key, true);
+		input_sync(input);
+		return true;
+	}
+
+	return false;
+}
+
+/**
+ * touch_overlay_process_event - process input events according to the overlay
+ * mapping. This function acts as a filter to release the calling driver from
+ * the events that are either related to overlay buttons or out of the overlay
+ * touchscreen area if defined.
+ * @list: pointer to the list that holds the segments
+ * @input: pointer to the input device associated to the event
+ * @x: pointer to the x coordinate (NULL if not available - no contact)
+ * @y: pointer to the y coordinate (NULL if not available - no contact)
+ * @slot: slot associated to the event
+ *
+ * Returns true if the event was processed (reported for valid key events
+ * and dropped for events outside the overlay touchscreen area) or false
+ * if the event must be processed by the caller. In that case this function
+ * shifts the (x,y) coordinates to the overlay touchscreen axis if required.
+ */
+bool touch_overlay_process_event(struct list_head *list,
+				 struct input_dev *input,
+				 u32 *x, u32 *y, u32 slot)
+{
+	struct touch_overlay_segment *segment;
+	struct list_head *ptr;
+
+	/*
+	 * buttons must be prioritized over overlay touchscreens to account for
+	 * overlappings e.g. a button inside the touchscreen area.
+	 */
+	list_for_each(ptr, list) {
+		segment = list_entry(ptr, struct touch_overlay_segment, list);
+		if (segment->btn &&
+		    touch_overlay_button_event(input, segment, x, y, slot)) {
+			return true;
+		}
+	}
+
+	/*
+	 * valid touch events on the overlay touchscreen are left for the client
+	 * to be processed/reported according to its (possibly) unique features.
+	 */
+	return !touch_overlay_mt_on_touchscreen(list, x, y);
+}
+EXPORT_SYMBOL(touch_overlay_process_event);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Helper functions for overlay objects on touch devices");
diff --git a/include/linux/input/touch-overlay.h b/include/linux/input/touch-overlay.h
new file mode 100644
index 000000000000..df974eb46dd4
--- /dev/null
+++ b/include/linux/input/touch-overlay.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Javier Carrasco <javier.carrasco@wolfvision.net>
+ */
+
+#ifndef _TOUCH_OVERLAY
+#define _TOUCH_OVERLAY
+
+#include <linux/types.h>
+
+struct input_dev;
+
+int touch_overlay_map(struct list_head *list, struct input_dev *keypad);
+
+void touch_overlay_get_touchscreen_abs(struct list_head *list, u16 *x, u16 *y);
+
+bool touch_overlay_mapped_touchscreen(struct list_head *list);
+
+bool touch_overlay_mapped_buttons(struct list_head *list);
+
+bool touch_overlay_process_event(struct list_head *list, struct input_dev *input,
+				 u32 *x, u32 *y, u32 slot);
+
+#endif

-- 
2.39.2


^ permalink raw reply related

* [PATCH v6 4/4] Input: st1232 - add touch overlays handling
From: Javier Carrasco @ 2023-12-20  8:39 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, Bastian Hecht, Michael Riesch
  Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v6-0-d8a605975153@wolfvision.net>

Use touch-overlay to support overlay objects such as buttons and a resized
frame defined in the device tree.

If buttons are provided, register an additional device to report key
events separately. A key event will be generated if the coordinates
of a touch event are within the area defined by the button properties.

Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
 drivers/input/touchscreen/st1232.c | 71 ++++++++++++++++++++++++++++++--------
 1 file changed, 57 insertions(+), 14 deletions(-)

diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index 6475084aee1b..5684cf04bfa1 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -22,6 +22,7 @@
 #include <linux/pm_qos.h>
 #include <linux/slab.h>
 #include <linux/types.h>
+#include <linux/input/touch-overlay.h>
 
 #define ST1232_TS_NAME	"st1232-ts"
 #define ST1633_TS_NAME	"st1633-ts"
@@ -56,7 +57,9 @@ struct st1232_ts_data {
 	struct touchscreen_properties prop;
 	struct dev_pm_qos_request low_latency_req;
 	struct gpio_desc *reset_gpio;
+	struct input_dev *overlay_keypad;
 	const struct st_chip_info *chip_info;
+	struct list_head touch_overlay_list;
 	int read_buf_len;
 	u8 *read_buf;
 };
@@ -130,6 +133,7 @@ static int st1232_ts_read_resolution(struct st1232_ts_data *ts, u16 *max_x,
 static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
 {
 	struct input_dev *input = ts->input_dev;
+	struct input_dev *keypad = ts->overlay_keypad;
 	struct input_mt_pos pos[ST_TS_MAX_FINGERS];
 	u8 z[ST_TS_MAX_FINGERS];
 	int slots[ST_TS_MAX_FINGERS];
@@ -138,14 +142,20 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
 
 	for (i = 0; i < ts->chip_info->max_fingers; i++) {
 		u8 *buf = &ts->read_buf[i * 4];
+		bool contact = buf[0] & BIT(7);
+		unsigned int x, y;
 
-		if (buf[0] & BIT(7)) {
-			unsigned int x = ((buf[0] & 0x70) << 4) | buf[1];
-			unsigned int y = ((buf[0] & 0x07) << 8) | buf[2];
-
-			touchscreen_set_mt_pos(&pos[n_contacts],
-					       &ts->prop, x, y);
+		if (contact) {
+			x = ((buf[0] & 0x70) << 4) | buf[1];
+			y = ((buf[0] & 0x07) << 8) | buf[2];
+		}
+		if (touch_overlay_process_event(&ts->touch_overlay_list, keypad,
+						contact ? &x : NULL,
+						contact ? &y : NULL, i))
+			continue;
 
+		if (contact) {
+			touchscreen_set_mt_pos(&pos[n_contacts], &ts->prop, x, y);
 			/* st1232 includes a z-axis / touch strength */
 			if (ts->chip_info->have_z)
 				z[n_contacts] = ts->read_buf[i + 6];
@@ -226,6 +236,7 @@ static int st1232_ts_probe(struct i2c_client *client)
 	const struct st_chip_info *match;
 	struct st1232_ts_data *ts;
 	struct input_dev *input_dev;
+	struct input_dev *overlay_keypad;
 	u16 max_x, max_y;
 	int error;
 
@@ -263,8 +274,13 @@ static int st1232_ts_probe(struct i2c_client *client)
 	if (!input_dev)
 		return -ENOMEM;
 
+	overlay_keypad = devm_input_allocate_device(&client->dev);
+	if (!overlay_keypad)
+		return -ENOMEM;
+
 	ts->client = client;
 	ts->input_dev = input_dev;
+	ts->overlay_keypad = overlay_keypad;
 
 	ts->reset_gpio = devm_gpiod_get_optional(&client->dev, NULL,
 						 GPIOD_OUT_HIGH);
@@ -286,24 +302,38 @@ static int st1232_ts_probe(struct i2c_client *client)
 
 	input_dev->name = "st1232-touchscreen";
 	input_dev->id.bustype = BUS_I2C;
+	overlay_keypad->name = "st1232-keypad";
+	overlay_keypad->id.bustype = BUS_I2C;
 
 	/* Wait until device is ready */
 	error = st1232_ts_wait_ready(ts);
 	if (error)
 		return error;
 
-	/* Read resolution from the chip */
-	error = st1232_ts_read_resolution(ts, &max_x, &max_y);
-	if (error) {
-		dev_err(&client->dev,
-			"Failed to read resolution: %d\n", error);
-		return error;
-	}
-
 	if (ts->chip_info->have_z)
 		input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
 				     ts->chip_info->max_area, 0, 0);
 
+	/* map overlay objects if defined in the device tree */
+	INIT_LIST_HEAD(&ts->touch_overlay_list);
+	error = touch_overlay_map(&ts->touch_overlay_list, ts->overlay_keypad);
+	if (error)
+		return error;
+
+	if (touch_overlay_mapped_touchscreen(&ts->touch_overlay_list)) {
+		/* Read resolution from the overlay touchscreen if defined */
+		touch_overlay_get_touchscreen_abs(&ts->touch_overlay_list,
+						  &max_x, &max_y);
+	} else {
+		/* Read resolution from the chip */
+		error = st1232_ts_read_resolution(ts, &max_x, &max_y);
+		if (error) {
+			dev_err(&client->dev,
+				"Failed to read resolution: %d\n", error);
+			return error;
+		}
+	}
+
 	input_set_abs_params(input_dev, ABS_MT_POSITION_X,
 			     0, max_x, 0, 0);
 	input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
@@ -335,6 +365,19 @@ static int st1232_ts_probe(struct i2c_client *client)
 		return error;
 	}
 
+	/* Register keypad input device if overlay buttons were defined */
+	if (touch_overlay_mapped_buttons(&ts->touch_overlay_list)) {
+		error = input_register_device(ts->overlay_keypad);
+		if (error) {
+			dev_err(&client->dev,
+				"Unable to register %s input device\n",
+				overlay_keypad->name);
+			return error;
+		}
+	} else {
+		input_free_device(ts->overlay_keypad);
+	}
+
 	i2c_set_clientdata(client, ts);
 
 	return 0;

-- 
2.39.2


^ permalink raw reply related

* [PATCH v6 3/4] dt-bindings: input: touchscreen: st1232: add touch-overlay example
From: Javier Carrasco @ 2023-12-20  8:39 UTC (permalink / raw)
  To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Henrik Rydberg, Bastian Hecht, Michael Riesch
  Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v6-0-d8a605975153@wolfvision.net>

The touch-overaly feature adds support for segments (touch areas) on the
touchscreen surface that represent overlays with clipped touchscreen
areas and printed buttons.

Add nodes for a clipped touchscreen and overlay buttons to the existing
example.

Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
 .../input/touchscreen/sitronix,st1232.yaml         | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
index 1d8ca19fd37a..e7ee7a0d74c4 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
@@ -37,6 +37,7 @@ unevaluatedProperties: false
 
 examples:
   - |
+    #include <dt-bindings/input/linux-event-codes.h>
     i2c {
             #address-cells = <1>;
             #size-cells = <0>;
@@ -46,5 +47,33 @@ examples:
                     reg = <0x55>;
                     interrupts = <2 0>;
                     gpios = <&gpio1 166 0>;
+
+                    touch-overlay {
+                            segment-0 {
+                                    label = "Touchscreen";
+                                    x-origin = <0>;
+                                    x-size = <240>;
+                                    y-origin = <40>;
+                                    y-size = <280>;
+                            };
+
+                            segment-1a {
+                                    label = "Camera light";
+                                    linux,code = <KEY_LIGHTS_TOGGLE>;
+                                    x-origin = <40>;
+                                    x-size = <40>;
+                                    y-origin = <0>;
+                                    y-size = <40>;
+                            };
+
+                            segment-2a {
+                                    label = "Power";
+                                    linux,code = <KEY_POWER>;
+                                    x-origin = <160>;
+                                    x-size = <40>;
+                                    y-origin = <0>;
+                                    y-size = <40>;
+                            };
+                    };
             };
     };

-- 
2.39.2


^ permalink raw reply related

* Re: [PATCH 2/3] HID: make ishtp_cl_bus_type const
From: srinivas pandruvada @ 2023-12-20 12:55 UTC (permalink / raw)
  To: Greg Kroah-Hartman, jikos, benjamin.tissoires; +Cc: linux-kernel, linux-input
In-Reply-To: <2023122048-tribute-quote-1b4d@gregkh>

On Wed, 2023-12-20 at 08:38 +0100, Greg Kroah-Hartman wrote:
> Now that the driver core can properly handle constant struct
> bus_type,
> move the ishtp_cl_bus_type variable to be a constant structure as
> well,
> placing it into read-only memory which can not be modified at
> runtime.
> 
> Cc: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Cc: Jiri Kosina <jikos@kernel.org>
> Cc: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

> ---
>  drivers/hid/intel-ish-hid/ishtp/bus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/intel-ish-hid/ishtp/bus.c
> b/drivers/hid/intel-ish-hid/ishtp/bus.c
> index 7fc738a22375..aa6cb033bb06 100644
> --- a/drivers/hid/intel-ish-hid/ishtp/bus.c
> +++ b/drivers/hid/intel-ish-hid/ishtp/bus.c
> @@ -378,7 +378,7 @@ static const struct dev_pm_ops
> ishtp_cl_bus_dev_pm_ops = {
>         .restore = ishtp_cl_device_resume,
>  };
>  
> -static struct bus_type ishtp_cl_bus_type = {
> +static const struct bus_type ishtp_cl_bus_type = {
>         .name           = "ishtp",
>         .dev_groups     = ishtp_cl_dev_groups,
>         .probe          = ishtp_cl_device_probe,


^ permalink raw reply

* Re: [PATCH v3 6/6] x86/vmware: Add TDX hypercall support
From: kernel test robot @ 2023-12-20 13:09 UTC (permalink / raw)
  To: Alexey Makhalov, linux-kernel, virtualization, bp, hpa,
	dave.hansen, mingo, tglx
  Cc: llvm, oe-kbuild-all, dmitry.torokhov, tzimmermann, pv-drivers,
	netdev, richardcochran, x86, dri-devel, horms, akaher, timothym,
	linux-graphics-maintainer, mripard, jsipek, linux-input, namit,
	kirill.shutemov, airlied, zackr
In-Reply-To: <20231219215751.9445-7-alexey.makhalov@broadcom.com>

Hi Alexey,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on dtor-input/next dtor-input/for-linus linus/master v6.7-rc6 next-20231220]
[cannot apply to tip/x86/vmware]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Alexey-Makhalov/x86-vmware-Move-common-macros-to-vmware-h/20231220-060028
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20231219215751.9445-7-alexey.makhalov%40broadcom.com
patch subject: [PATCH v3 6/6] x86/vmware: Add TDX hypercall support
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20231220/202312202020.5O1T2aSk-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231220/202312202020.5O1T2aSk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202312202020.5O1T2aSk-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> arch/x86/kernel/cpu/vmware.c:439:40: warning: format specifies type 'unsigned int' but the argument has type 'unsigned long' [-Wformat]
                   pr_warn("Out of range command %x\n", cmd);
                                                 ~~     ^~~
                                                 %lx
   include/linux/printk.h:508:37: note: expanded from macro 'pr_warn'
           printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
                                      ~~~     ^~~~~~~~~~~
   include/linux/printk.h:455:60: note: expanded from macro 'printk'
   #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
                                                       ~~~    ^~~~~~~~~~~
   include/linux/printk.h:427:19: note: expanded from macro 'printk_index_wrap'
                   _p_func(_fmt, ##__VA_ARGS__);                           \
                           ~~~~    ^~~~~~~~~~~
   1 warning generated.


vim +439 arch/x86/kernel/cpu/vmware.c

   430	
   431	#ifdef CONFIG_INTEL_TDX_GUEST
   432	unsigned long vmware_tdx_hypercall(unsigned long cmd,
   433					   struct tdx_module_args *args)
   434	{
   435		if (!hypervisor_is_type(X86_HYPER_VMWARE))
   436			return 0;
   437	
   438		if (cmd & ~VMWARE_CMD_MASK) {
 > 439			pr_warn("Out of range command %x\n", cmd);
   440			return 0;
   441		}
   442	
   443		args->r10 = VMWARE_TDX_VENDOR_LEAF;
   444		args->r11 = VMWARE_TDX_HCALL_FUNC;
   445		args->r12 = VMWARE_HYPERVISOR_MAGIC;
   446		args->r13 = cmd;
   447	
   448		__tdx_hypercall(args);
   449	
   450		return args->r12;
   451	}
   452	EXPORT_SYMBOL_GPL(vmware_tdx_hypercall);
   453	#endif
   454	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH] input: uinput: Drop checks for abs_min > abs_max
From: Paul Cercueil @ 2023-12-20 13:39 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Peter Hutterer, Chris Morgan, linux-input, svv, biswarupp,
	contact, Chris Morgan
In-Reply-To: <ZYJJC-ID4SyHhuuA@google.com>

Hi Dmitry,

Le mardi 19 décembre 2023 à 17:53 -0800, Dmitry Torokhov a écrit :
> Hi Paul,
> 
> On Wed, Dec 20, 2023 at 01:38:39AM +0100, Paul Cercueil wrote:
> > Hi Peter,
> > 
> > Le mercredi 20 décembre 2023 à 09:51 +1000, Peter Hutterer a
> > écrit :
> > > On Mon, Dec 18, 2023 at 11:16:53AM -0600, Chris Morgan wrote:
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > 
> > > > Stop checking if the minimum abs value is greater than the
> > > > maximum
> > > > abs
> > > > value. When the axis is inverted this condition is allowed.
> > > > Without
> > > > relaxing this check, it is not possible to use uinput on
> > > > devices in
> > > > userspace with an inverted axis, such as the adc-joystick found
> > > > on
> > > > many handheld gaming devices.
> > > 
> > > As mentioned in the other thread [1] a fair bit of userspace
> > > relies
> > > on
> > > that general assumption so removing it will likely cause all
> > > sorts of
> > > issues.
> > 
> > There is some userspace that works with it though, so why restrict
> > it
> > artificially?
> > 
> > The fact that some other userspace code wouldn't work with it
> > sounds a
> > bit irrelevant. They just never encountered that min>max usage
> > before.
> > 
> > And removing this check won't cause all sort of issues, why would
> > it?
> > It's not like the current software actively probes min>max and
> > crash
> > badly if it doesn't return -EINVAL...
> 
> It will cause weird movements because calculations expect min be the
> minimum, and max the maximum, and not encode left/right or up/down.
> Putting this into adc joystick binding was a mistake.

I don't see why it was a mistake, it's only one of the ways to specify
that the axis is inverted. This information is between the firmware
(DT) and the kernel, that doesn't mean the information has to be
relayed as-is to the userspace.

Unlike what you wrote in your other answer, when talking about input
the kernel doesn't really normalize anything - it gives you the min/max
values, and the raw samples, not normalized samples (they don't get
translated to a pre-specified range, or even clamped).

I don't really like the idea of having the driver tamper with the
samples, but if the specification really is that max>min, then it would
be up to evdev/joydev (if the individual drivers are allowed min>max)
or adc-joystick (if they are not) to process the samples.

Cheers,
-Paul

> This is the definition of absinfo:
> 
> /**
>  * struct input_absinfo - used by EVIOCGABS/EVIOCSABS ioctls
>  * @value: latest reported value for the axis.
>  * @minimum: specifies minimum value for the axis.
>  * @maximum: specifies maximum value for the axis.
>  * @fuzz: specifies fuzz value that is used to filter noise from
>  *	the event stream.
>  * @flat: values that are within this value will be discarded by
>  *	joydev interface and reported as 0 instead.
>  * @resolution: specifies resolution for the values reported for
>  *	the axis.
>  *
>  * Note that input core does not clamp reported values to the
>  * [minimum, maximum] limits, such task is left to userspace.
> ...
>  */
> 
> (We should change wording of the last sentence to "... does not
> always
> clamp ..." since when we do inversion/swap we do clamp values.)
> 
> And note that the userspace that can handle swapped min and max will
> work fine if the kernel provides normalized data, but other software
> will/may not work.
> 
> Thanks.
> 


^ permalink raw reply

* Re: [PATCH] HID: sensor-hub: Enable hid core report processing for all devices
From: Jonathan Cameron @ 2023-12-20 14:52 UTC (permalink / raw)
  To: Yauhen Kharuzhy
  Cc: linux-input, linux-iio, Daniel Thompson, linux-kernel,
	Jiri Kosina, Srinivas Pandruvada, Benjamin Tissoires
In-Reply-To: <20231219231503.1506801-1-jekhor@gmail.com>

On Wed, 20 Dec 2023 01:15:03 +0200
Yauhen Kharuzhy <jekhor@gmail.com> wrote:

> After the commit 666cf30a589a ("HID: sensor-hub: Allow multi-function
> sensor devices") hub devices are claimed by hidraw driver in hid_connect().
> This causes stoppping of processing HID reports by hid core due to
> optimization.
> 
> In such case, the hid-sensor-custom driver cannot match a known custom
> sensor in hid_sensor_custom_get_known() because it try to check custom
> properties which weren't filled from the report because hid core didn't
> parsed it.
> 
> As result, custom sensors like hinge angle sensor and LISS sensors
> don't work.
> 
> Mark the sensor hub devices claimed by some driver to avoid hidraw-related
> optimizations.
> 
> Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
Fixes tag?

> ---
>  drivers/hid/hid-sensor-hub.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> index 2eba152e8b90..26e93a331a51 100644
> --- a/drivers/hid/hid-sensor-hub.c
> +++ b/drivers/hid/hid-sensor-hub.c
> @@ -632,7 +632,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
>  	}
>  	INIT_LIST_HEAD(&hdev->inputs);
>  
> -	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> +	ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT | HID_CONNECT_DRIVER);
>  	if (ret) {
>  		hid_err(hdev, "hw start failed\n");
>  		return ret;


^ permalink raw reply

* Re: [PATCH] HID: sensor-hub: Enable hid core report processing for all devices
From: Yauhen Kharuzhy @ 2023-12-20 15:04 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-input, linux-iio, Daniel Thompson, linux-kernel,
	Jiri Kosina, Srinivas Pandruvada, Benjamin Tissoires
In-Reply-To: <20231220145229.020abe62@jic23-huawei>

ср, 20 дек. 2023 г. в 16:52, Jonathan Cameron <jic23@kernel.org>:
>
> On Wed, 20 Dec 2023 01:15:03 +0200
> Yauhen Kharuzhy <jekhor@gmail.com> wrote:
>
> > After the commit 666cf30a589a ("HID: sensor-hub: Allow multi-function
> > sensor devices") hub devices are claimed by hidraw driver in hid_connect().
> > This causes stoppping of processing HID reports by hid core due to
> > optimization.
> >
> > In such case, the hid-sensor-custom driver cannot match a known custom
> > sensor in hid_sensor_custom_get_known() because it try to check custom
> > properties which weren't filled from the report because hid core didn't
> > parsed it.
> >
> > As result, custom sensors like hinge angle sensor and LISS sensors
> > don't work.
> >
> > Mark the sensor hub devices claimed by some driver to avoid hidraw-related
> > optimizations.
> >
> > Signed-off-by: Yauhen Kharuzhy <jekhor@gmail.com>
> Fixes tag?

Fixes: 666cf30a589a ("HID: sensor-hub: Allow multi-function sensor devices")

>
> > ---
> >  drivers/hid/hid-sensor-hub.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
> > index 2eba152e8b90..26e93a331a51 100644
> > --- a/drivers/hid/hid-sensor-hub.c
> > +++ b/drivers/hid/hid-sensor-hub.c
> > @@ -632,7 +632,7 @@ static int sensor_hub_probe(struct hid_device *hdev,
> >       }
> >       INIT_LIST_HEAD(&hdev->inputs);
> >
> > -     ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> > +     ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT | HID_CONNECT_DRIVER);
> >       if (ret) {
> >               hid_err(hdev, "hw start failed\n");
> >               return ret;
>


-- 
Yauhen Kharuzhy

^ permalink raw reply

* Re: [PATCH v2 1/3] iio: hid-sensor-als: Allocate channels dynamically
From: Jonathan Cameron @ 2023-12-20 15:24 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: jikos, lars, Basavaraj.Natikar, linux-input, linux-iio,
	linux-kernel
In-Reply-To: <20231218203026.1156375-2-srinivas.pandruvada@linux.intel.com>

On Mon, 18 Dec 2023 12:30:24 -0800
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> Instead of assuming that every channel defined statically by
> als_channels[] is present, allocate dynamically based on presence of the
> respective usage id in the descriptor. This will allow to register ALS
> with limited channel support. Append the timestamp as the last channel.
> 
> There is no intentional function changes done.
> 
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Hi Srinivas,

No huge rush on this as I'll not have the revert in my upstream now
until after the merge window + may not have a chance for a second pull
request anyway.

A few comments inline,

Jonathan

> ---
> v2:
> New change
> 
>  drivers/iio/light/hid-sensor-als.c | 57 ++++++++++++++++++------------
>  1 file changed, 35 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index 5cd27f04b45e..e57ad1946b56 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -236,14 +236,21 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
>  
>  /* Parse report which is specific to an usage id*/
>  static int als_parse_report(struct platform_device *pdev,
> -				struct hid_sensor_hub_device *hsdev,
> -				struct iio_chan_spec *channels,
> -				unsigned usage_id,
> -				struct als_state *st)
> +			    struct hid_sensor_hub_device *hsdev,
> +			    struct iio_chan_spec **channels_out,
> +			    int *size_channels_out,
> +			    unsigned int usage_id,
> +			    struct als_state *st)

Align with opening bracket.  Also, try in general to avoid white space changes
when changing anything else.  It would be easier to see what actually changed
here without that.

>  {
> -	int ret;
> +	struct iio_chan_spec *channels;
> +	int ret, index = 0;
>  	int i;
>  
> +	channels = devm_kcalloc(&pdev->dev, ARRAY_SIZE(als_channels),
> +				sizeof(als_channels), GFP_KERNEL);

Given it's a fixed size (which is fine even though you might not use it
all), can you just make it part of the iio_priv() structure
and avoid need for handling the allocation here?


> +	if (!channels)
> +		return -ENOMEM;
> +
>  	for (i = 0; i <= CHANNEL_SCAN_INDEX_ILLUM; ++i) {
>  		ret = sensor_hub_input_get_attribute_info(hsdev,
>  						HID_INPUT_REPORT,
> @@ -251,9 +258,11 @@ static int als_parse_report(struct platform_device *pdev,
>  						HID_USAGE_SENSOR_LIGHT_ILLUM,
>  						&st->als[i]);
>  		if (ret < 0)
> -			return ret;
> -		als_adjust_channel_bit_mask(channels, i, st->als[i].size);
> +			break;
>  
> +		channels[i] = als_channels[i];

channels[index] = als_channels[i]? Might be gaps. What you currently have
only works if the 'present channels' are all at the start.

> +		als_adjust_channel_bit_mask(channels, i, st->als[i].size);

Would use index not i.

> +		++index;
>  		dev_dbg(&pdev->dev, "als %x:%x\n", st->als[i].index,
>  			st->als[i].report_id);
>  	}
> @@ -262,17 +271,24 @@ static int als_parse_report(struct platform_device *pdev,
>  				&st->als[CHANNEL_SCAN_INDEX_INTENSITY],
>  				&st->scale_pre_decml, &st->scale_post_decml);
>  
> -	return ret;
> +	*channels_out = channels;
> +	*size_channels_out = index;
> +
> +	if (!index)
> +		ret = -ENODEV;
> +
> +	return 0;
>  }
>  
>  /* Function to initialize the processing for usage id */
>  static int hid_als_probe(struct platform_device *pdev)
>  {
> -	int ret = 0;
> +	int ret = 0, max_channel_index;
>  	static const char *name = "als";
>  	struct iio_dev *indio_dev;
>  	struct als_state *als_state;
>  	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
> +	struct iio_chan_spec *channels;
>  
>  	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(struct als_state));
>  	if (!indio_dev)
> @@ -293,24 +309,21 @@ static int hid_als_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> -	indio_dev->channels = devm_kmemdup(&pdev->dev, als_channels,
> -					   sizeof(als_channels), GFP_KERNEL);
> -	if (!indio_dev->channels) {
> -		dev_err(&pdev->dev, "failed to duplicate channels\n");
> -		return -ENOMEM;
> -	}
> -
> -	ret = als_parse_report(pdev, hsdev,
> -			       (struct iio_chan_spec *)indio_dev->channels,
> -			       hsdev->usage,
> -			       als_state);
> +	ret = als_parse_report(pdev, hsdev, &channels, &max_channel_index,
> +			       hsdev->usage, als_state);
>  	if (ret) {
>  		dev_err(&pdev->dev, "failed to setup attributes\n");
>  		return ret;
>  	}
>  
> -	indio_dev->num_channels =
> -				ARRAY_SIZE(als_channels);
> +	/* Add timestamp channel */
> +	channels[max_channel_index] = als_channels[CHANNEL_SCAN_INDEX_TIMESTAMP];
> +	channels[max_channel_index].scan_index = max_channel_index;
> +
> +	/* +1 for adding timestamp channel */
> +	indio_dev->num_channels = max_channel_index + 1;
> +
> +	indio_dev->channels = channels;
>  	indio_dev->info = &als_info;
>  	indio_dev->name = name;
>  	indio_dev->modes = INDIO_DIRECT_MODE;


^ permalink raw reply

* Re: [PATCH v2 3/3] iio: hid-sensor-als: Add light chromaticity support
From: Jonathan Cameron @ 2023-12-20 15:29 UTC (permalink / raw)
  To: Srinivas Pandruvada
  Cc: jikos, lars, Basavaraj.Natikar, linux-input, linux-iio,
	linux-kernel
In-Reply-To: <20231218203026.1156375-4-srinivas.pandruvada@linux.intel.com>

On Mon, 18 Dec 2023 12:30:26 -0800
Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> wrote:

> From: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> 
> In most cases, ambient color sensors also support the x and y light
> colors, which represent the coordinates on the CIE 1931 chromaticity
> diagram. Thus, add light chromaticity x and y.
> 
> Signed-off-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Is it not possible to unify the support to be a single pass over the
channels array that just checks each one for availability and copies
the channel in if is found?  Tweaked slightly to deal with
the pair of chromaticity channels.

Jonathan

> ---
> v2:
> Original patch from Basavaraj Natikar <Basavaraj.Natikar@amd.com> is
> modified to prevent failure when the new usage id is not found in the
> descriptor.
> 
>  drivers/iio/light/hid-sensor-als.c | 68 +++++++++++++++++++++++++++++-
>  include/linux/hid-sensor-ids.h     |  3 ++
>  2 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index 8d6beacc338a..6e2793fa515c 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
> @@ -17,6 +17,8 @@ enum {
>  	CHANNEL_SCAN_INDEX_INTENSITY,
>  	CHANNEL_SCAN_INDEX_ILLUM,
>  	CHANNEL_SCAN_INDEX_COLOR_TEMP,
> +	CHANNEL_SCAN_INDEX_CHROMATICITY_X,
> +	CHANNEL_SCAN_INDEX_CHROMATICITY_Y,
>  	CHANNEL_SCAN_INDEX_MAX
>  };
>  
> @@ -76,6 +78,30 @@ static const struct iio_chan_spec als_channels[] = {
>  		BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
>  		.scan_index = CHANNEL_SCAN_INDEX_COLOR_TEMP,
>  	},
> +	{
> +		.type = IIO_CHROMATICITY,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_X,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
> +		BIT(IIO_CHAN_INFO_SCALE) |
> +		BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +		BIT(IIO_CHAN_INFO_HYSTERESIS) |
> +		BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
> +		.scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_X,
> +	},
> +	{
> +		.type = IIO_CHROMATICITY,
> +		.modified = 1,
> +		.channel2 = IIO_MOD_Y,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OFFSET) |
> +		BIT(IIO_CHAN_INFO_SCALE) |
> +		BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +		BIT(IIO_CHAN_INFO_HYSTERESIS) |
> +		BIT(IIO_CHAN_INFO_HYSTERESIS_RELATIVE),
> +		.scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_Y,
> +	},
>  	IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP)
>  };
>  
> @@ -119,6 +145,16 @@ static int als_read_raw(struct iio_dev *indio_dev,
>  			min = als_state->als[chan->scan_index].logical_minimum;
>  			address = HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE;
>  			break;
> +		case  CHANNEL_SCAN_INDEX_CHROMATICITY_X:
> +			report_id = als_state->als[chan->scan_index].report_id;
> +			min = als_state->als[chan->scan_index].logical_minimum;
> +			address = HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X;
> +			break;
> +		case  CHANNEL_SCAN_INDEX_CHROMATICITY_Y:
> +			report_id = als_state->als[chan->scan_index].report_id;
> +			min = als_state->als[chan->scan_index].logical_minimum;
> +			address = HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y;
> +			break;
>  		default:
>  			report_id = -1;
>  			break;
> @@ -243,6 +279,14 @@ static int als_capture_sample(struct hid_sensor_hub_device *hsdev,
>  		als_state->scan.illum[CHANNEL_SCAN_INDEX_COLOR_TEMP] = sample_data;
>  		ret = 0;
>  		break;
> +	case HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X:
> +		als_state->scan.illum[CHANNEL_SCAN_INDEX_CHROMATICITY_X] = sample_data;
> +		ret = 0;
> +		break;
> +	case HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y:
> +		als_state->scan.illum[CHANNEL_SCAN_INDEX_CHROMATICITY_Y] = sample_data;
> +		ret = 0;
> +		break;
>  	case HID_USAGE_SENSOR_TIME_TIMESTAMP:
>  		als_state->timestamp = hid_sensor_convert_timestamp(&als_state->common_attributes,
>  								    *(s64 *)raw_data);
> @@ -303,11 +347,33 @@ static int als_parse_report(struct platform_device *pdev,
>  		st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].index,
>  		st->als[CHANNEL_SCAN_INDEX_COLOR_TEMP].report_id);
>  
> +skip_temp_channel:
> +	for (i = 0; i < 2; i++) {
> +		int next_scan_index = CHANNEL_SCAN_INDEX_CHROMATICITY_X + i;
> +
> +		ret = sensor_hub_input_get_attribute_info(hsdev,
> +				HID_INPUT_REPORT, usage_id,
> +				HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X + i,
> +				&st->als[next_scan_index]);
> +		if (ret < 0)
> +			goto skip_chromaticity_channel;
> +
> +		channels[index++] = als_channels[CHANNEL_SCAN_INDEX_CHROMATICITY_X + i];
> +
> +		als_adjust_channel_bit_mask(channels,
> +					CHANNEL_SCAN_INDEX_CHROMATICITY_X + i,
> +					st->als[next_scan_index].size);
> +
> +		dev_dbg(&pdev->dev, "als %x:%x\n",
> +			st->als[next_scan_index].index,
> +			st->als[next_scan_index].report_id);
> +	}
> +
>  	st->scale_precision = hid_sensor_format_scale(usage_id,
>  				&st->als[CHANNEL_SCAN_INDEX_INTENSITY],
>  				&st->scale_pre_decml, &st->scale_post_decml);
>  
> -skip_temp_channel:
> +skip_chromaticity_channel:
>  	*channels_out = channels;
>  	*size_channels_out = index;
>  
> diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
> index 8af4fb3e0254..6730ee900ee1 100644
> --- a/include/linux/hid-sensor-ids.h
> +++ b/include/linux/hid-sensor-ids.h
> @@ -22,6 +22,9 @@
>  #define HID_USAGE_SENSOR_DATA_LIGHT				0x2004d0
>  #define HID_USAGE_SENSOR_LIGHT_ILLUM				0x2004d1
>  #define HID_USAGE_SENSOR_LIGHT_COLOR_TEMPERATURE		0x2004d2
> +#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY			0x2004d3
> +#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY_X			0x2004d4
> +#define HID_USAGE_SENSOR_LIGHT_CHROMATICITY_Y			0x2004d5
>  
>  /* PROX (200011) */
>  #define HID_USAGE_SENSOR_PROX                                   0x200011


^ permalink raw reply

* Re: [PATCH] Input: SoC button array: add mapping for airplane mode button
From: Werner Sembach @ 2023-12-20 15:38 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Christoffer Sandberg, linux-input, linux-kernel
In-Reply-To: <20231215171718.80229-1-wse@tuxedocomputers.com>

Am 15.12.23 um 18:17 schrieb Werner Sembach:
> From: Christoffer Sandberg <cs@tuxedo.de>
>
> This add a mapping for the airplane mode button on the TUXEDO Pulse Gen3.
>
> While it is physically a key it behaves more like a switch, sending a key
> down on first press and a key up on 2nd press. Therefor the switch event is
> used here. Besides this behaviour it uses the HID usage-id 0xc6 (Wireless
> Radio Button) and not 0xc8 (Wireless Radio Slider Switch), but since
> neither 0xc6 nor 0xc8 are currently implemented at all in soc_button_array
> this not to standard behaviour is not put behind a quirk for the moment.
>
> Signed-off-by: Christoffer Sandberg <cs@tuxedo.de>
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>

Cc: stable@vger.kernel.org

> ---
>   drivers/input/misc/soc_button_array.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> index 08bcee3d6bcca..f6d060377d189 100644
> --- a/drivers/input/misc/soc_button_array.c
> +++ b/drivers/input/misc/soc_button_array.c
> @@ -299,6 +299,11 @@ static int soc_button_parse_btn_desc(struct device *dev,
>   		info->name = "power";
>   		info->event_code = KEY_POWER;
>   		info->wakeup = true;
> +	} else if (upage == 0x01 && usage == 0xc6) {
> +		info->name = "airplane mode switch";
> +		info->event_type = EV_SW;
> +		info->event_code = SW_RFKILL_ALL;
> +		info->active_low = false;
>   	} else if (upage == 0x01 && usage == 0xca) {
>   		info->name = "rotation lock switch";
>   		info->event_type = EV_SW;

^ permalink raw reply

* Re: [PATCH] Input: SoC button array: add mapping for airplane mode button
From: Werner Sembach @ 2023-12-20 15:45 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Christoffer Sandberg, linux-input, linux-kernel
In-Reply-To: <20231215171718.80229-1-wse@tuxedocomputers.com>

Am 15.12.23 um 18:17 schrieb Werner Sembach:
> From: Christoffer Sandberg <cs@tuxedo.de>
>
> This add a mapping for the airplane mode button on the TUXEDO Pulse Gen3.
>
> While it is physically a key it behaves more like a switch, sending a key
> down on first press and a key up on 2nd press. Therefor the switch event is
> used here. Besides this behaviour it uses the HID usage-id 0xc6 (Wireless
> Radio Button) and not 0xc8 (Wireless Radio Slider Switch), but since
> neither 0xc6 nor 0xc8 are currently implemented at all in soc_button_array
> this not to standard behaviour is not put behind a quirk for the moment.
>
> Signed-off-by: Christoffer Sandberg <cs@tuxedo.de>
> Signed-off-by: Werner Sembach <wse@tuxedocomputers.com>
For reference: https://bugzilla.kernel.org/show_bug.cgi?id=214259
> ---
>   drivers/input/misc/soc_button_array.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
> index 08bcee3d6bcca..f6d060377d189 100644
> --- a/drivers/input/misc/soc_button_array.c
> +++ b/drivers/input/misc/soc_button_array.c
> @@ -299,6 +299,11 @@ static int soc_button_parse_btn_desc(struct device *dev,
>   		info->name = "power";
>   		info->event_code = KEY_POWER;
>   		info->wakeup = true;
> +	} else if (upage == 0x01 && usage == 0xc6) {
> +		info->name = "airplane mode switch";
> +		info->event_type = EV_SW;
> +		info->event_code = SW_RFKILL_ALL;
> +		info->active_low = false;
>   	} else if (upage == 0x01 && usage == 0xca) {
>   		info->name = "rotation lock switch";
>   		info->event_type = EV_SW;

^ permalink raw reply

* Re: [RFC PATCH 4/5] input: add onkey driver for Marvell 88PM88X PMICs
From: Dmitry Torokhov @ 2023-12-20 23:33 UTC (permalink / raw)
  To: Karel Balej
  Cc: Karel Balej, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Lee Jones, linux-input, devicetree, linux-kernel,
	Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <20231217131838.7569-5-karelb@gimli.ms.mff.cuni.cz>

Hi Karel,

On Sun, Dec 17, 2023 at 02:17:02PM +0100, Karel Balej wrote:
> From: Karel Balej <balejk@matfyz.cz>
> 
> The Marvell 88PM88X PMICs provide onkey among other things. Add client
> driver to handle it. The driver currently only provides a basic support
> omitting additional functions found in the vendor version, such as long
> onkey and GPIO integration.
> 
> Signed-off-by: Karel Balej <balejk@matfyz.cz>
> ---
>  drivers/input/misc/88pm88x-onkey.c | 103 +++++++++++++++++++++++++++++
>  drivers/input/misc/Kconfig         |  10 +++
>  drivers/input/misc/Makefile        |   1 +
>  3 files changed, 114 insertions(+)
>  create mode 100644 drivers/input/misc/88pm88x-onkey.c
> 
> diff --git a/drivers/input/misc/88pm88x-onkey.c b/drivers/input/misc/88pm88x-onkey.c
> new file mode 100644
> index 000000000000..0d6056a3cab2
> --- /dev/null
> +++ b/drivers/input/misc/88pm88x-onkey.c
> @@ -0,0 +1,103 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/kernel.h>
> +#include <linux/input.h>
> +#include <linux/regmap.h>
> +#include <linux/irq.h>
> +#include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>

Please sort alphabetically.

> +
> +#include <linux/mfd/88pm88x.h>
> +
> +struct pm88x_onkey {
> +	struct input_dev *idev;
> +	struct pm88x_chip *chip;
> +	int irq;

Since you are using devm to request interrupt I do not think you need to
store it here.

> +};
> +
> +static irqreturn_t pm88x_onkey_irq_handler(int irq, void *data)
> +{
> +	struct pm88x_onkey *onkey = data;
> +	unsigned int val;
> +	int ret = 0;

"error". Also no need to initialize it to 0.

> +
> +	ret = regmap_read(onkey->chip->regmaps[PM88X_REGMAP_BASE], PM88X_REG_STATUS1, &val);

I still prefer to keep withing 80 columns, unless longer lines are a
clear win.

> +	if (ret) {
> +		dev_err(onkey->idev->dev.parent, "Failed to read status: %d\n", ret);

Maybe have "dev" in onkey instead of poking through input?

> +		return IRQ_NONE;
> +	}
> +	val &= PM88X_ONKEY_STS1;
> +
> +	input_report_key(onkey->idev, KEY_POWER, val);
> +	input_sync(onkey->idev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int pm88x_onkey_probe(struct platform_device *pdev)
> +{
> +	struct pm88x_chip *chip = dev_get_drvdata(pdev->dev.parent);
> +	struct pm88x_onkey *onkey;
> +	int err;
> +
> +	onkey = devm_kzalloc(&pdev->dev, sizeof(*onkey), GFP_KERNEL);
> +	if (!onkey)
> +		return -ENOMEM;
> +
> +	onkey->chip = chip;
> +
> +	onkey->irq = platform_get_irq(pdev, 0);
> +	if (onkey->irq < 0) {
> +		dev_err(&pdev->dev, "Failed to get IRQ\n");
> +		return -EINVAL;

Do not clobber the return from platform_get_irq(). So "return
onkey->irq" (or simply irq if you drop it from onkey and use temporary).

> +	}
> +
> +	onkey->idev = devm_input_allocate_device(&pdev->dev);
> +	if (!onkey->idev) {
> +		dev_err(&pdev->dev, "Failed to allocate input device\n");
> +		return -ENOMEM;
> +	}
> +
> +	onkey->idev->name = "88pm88x-onkey";
> +	onkey->idev->phys = "88pm88x-onkey/input0";
> +	onkey->idev->id.bustype = BUS_I2C;
> +	onkey->idev->dev.parent = &pdev->dev;

No need to do this since you are using devm_input_allocate_device().

> +	onkey->idev->evbit[0] = BIT_MASK(EV_KEY);
> +	onkey->idev->keybit[BIT_WORD(KEY_POWER)] = BIT_MASK(KEY_POWER);

Please use

	input_set_capability(onkey->idev, EV_KEY, KEY_POWER);

> +
> +	err = devm_request_threaded_irq(&pdev->dev, onkey->irq, NULL, pm88x_onkey_irq_handler,
> +			IRQF_ONESHOT | IRQF_NO_SUSPEND, "onkey", onkey);

Please align arguments.

> +	if (err) {
> +		dev_err(&pdev->dev, "Failed to request IRQ: %d\n", err);

I was persuaded to stop my crusade against dev_err_probe() so you may
use it here.

> +		return err;
> +	}
> +
> +	err = input_register_device(onkey->idev);
> +	if (err) {
> +		dev_err(&pdev->dev, "Failed to register input device: %d\n", err);
> +		return err;
> +	}
> +
> +	device_init_wakeup(&pdev->dev, 1);

Are there cases where we woudl not want wakeup? Maybe use standard
"wakeup-source" property?

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id pm88x_onkey_of_match[] = {
> +	{ .compatible = "marvell,88pm88x-onkey", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, pm88x_onkey_of_match);
> +
> +static struct platform_driver pm88x_onkey_driver = {
> +	.driver = {
> +		.name = "88pm88x-onkey",
> +		.of_match_table = of_match_ptr(pm88x_onkey_of_match),

Given that you are not guarding pm88x_onkey_of_match definition with
#ifdef CONFIG_OF you shoudl not use of_match_ptr().

Thanks.

-- 
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