diff mbox series

[v2] HID: amd_sfh: Add support for tablet-mode-switch sensor

Message ID 20221218214723.14735-1-adrian@freund.io (mailing list archive)
State New, archived
Delegated to: Jiri Kosina
Headers show
Series [v2] HID: amd_sfh: Add support for tablet-mode-switch sensor | expand

Commit Message

Adrian Freund Dec. 18, 2022, 9:47 p.m. UTC
This patch adds support for the tablet mode switch sensors on
convertible devices where that sensor is managed by AMD SFH, like the
Asus Flow X13 and the Lenovo ThinkPad L13 Yoga Gen2 (AMD).

Co-developed-by: Ivan Dovgal <iv.dovg@gmail.com>
Signed-off-by: Ivan Dovgal <iv.dovg@gmail.com>
Co-developed-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Adrian Freund <adrian@freund.io>
---
v2:
* Fixed build warning reported by kernel test robot <lkp@intel.com>

 drivers/hid/amd-sfh-hid/amd_sfh_client.c      |  2 ++
 drivers/hid/amd-sfh-hid/amd_sfh_hid.h         |  2 +-
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.c        |  4 +++
 drivers/hid/amd-sfh-hid/amd_sfh_pcie.h        |  1 +
 .../hid_descriptor/amd_sfh_hid_desc.c         | 27 +++++++++++++++++++
 .../hid_descriptor/amd_sfh_hid_desc.h         |  7 +++++
 .../hid_descriptor/amd_sfh_hid_report_desc.h  | 21 +++++++++++++++
 7 files changed, 63 insertions(+), 1 deletion(-)

Comments

Jiri Kosina Jan. 18, 2023, 8:49 a.m. UTC | #1
On Sun, 18 Dec 2022, Adrian Freund wrote:

> This patch adds support for the tablet mode switch sensors on
> convertible devices where that sensor is managed by AMD SFH, like the
> Asus Flow X13 and the Lenovo ThinkPad L13 Yoga Gen2 (AMD).
> 
> Co-developed-by: Ivan Dovgal <iv.dovg@gmail.com>
> Signed-off-by: Ivan Dovgal <iv.dovg@gmail.com>
> Co-developed-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Luke D. Jones <luke@ljones.dev>
> Signed-off-by: Adrian Freund <adrian@freund.io>

Basavaraj, could you please review this? Thanks,
Basavaraj Natikar Jan. 18, 2023, 9:06 a.m. UTC | #2
On 1/18/2023 2:19 PM, Jiri Kosina wrote:
> On Sun, 18 Dec 2022, Adrian Freund wrote:
>
>> This patch adds support for the tablet mode switch sensors on
>> convertible devices where that sensor is managed by AMD SFH, like the
>> Asus Flow X13 and the Lenovo ThinkPad L13 Yoga Gen2 (AMD).
>>
>> Co-developed-by: Ivan Dovgal <iv.dovg@gmail.com>
>> Signed-off-by: Ivan Dovgal <iv.dovg@gmail.com>
>> Co-developed-by: Luke D. Jones <luke@ljones.dev>
>> Signed-off-by: Luke D. Jones <luke@ljones.dev>
>> Signed-off-by: Adrian Freund <adrian@freund.io>

Whole patch does not take any input data validation from any sensors

Please specify what are valid values?

Thanks,
--
Basavaraj
Adrian Freund Jan. 18, 2023, 9:13 a.m. UTC | #3
On 1/18/23 09:49, Jiri Kosina wrote:
> On Sun, 18 Dec 2022, Adrian Freund wrote:
>
>> This patch adds support for the tablet mode switch sensors on
>> convertible devices where that sensor is managed by AMD SFH, like the
>> Asus Flow X13 and the Lenovo ThinkPad L13 Yoga Gen2 (AMD).
>>
>> Co-developed-by: Ivan Dovgal <iv.dovg@gmail.com>
>> Signed-off-by: Ivan Dovgal <iv.dovg@gmail.com>
>> Co-developed-by: Luke D. Jones <luke@ljones.dev>
>> Signed-off-by: Luke D. Jones <luke@ljones.dev>
>> Signed-off-by: Adrian Freund <adrian@freund.io>
> Basavaraj, could you please review this? Thanks,
>
I think this doesn't need a review right now. I recently found some big 
problems with the patch recently and am working on an updated version. I 
think reviewing once I send v3 should be enough.

In short: This patch generates nonsensical HID messages that don't even 
contain the sensor state.
It still makes the tablet-mode-switch work on some but not all devices 
due to a mechanism I don't fully understand yet. Somehow ACPI devices 
that previously did nothing suddenly work with this patch and are then 
getting picked up by various platform drivers depending on the device. 
This driver however doesn't properly handle the actual data directly 
from the sensor.

---
Adrian
diff mbox series

Patch

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
index 8275bba63611..83dd0402933c 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
@@ -146,6 +146,8 @@  static const char *get_sensor_name(int idx)
 		return "gyroscope";
 	case mag_idx:
 		return "magnetometer";
+	case tms_idx:
+		return "tablet-mode-switch";
 	case als_idx:
 		return "ALS";
 	case HPD_IDX:
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
index 3754fb423e3a..f10ec16bb8aa 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
@@ -11,7 +11,7 @@ 
 #ifndef AMDSFH_HID_H
 #define AMDSFH_HID_H
 
-#define MAX_HID_DEVICES		5
+#define MAX_HID_DEVICES		6
 #define AMD_SFH_HID_VENDOR	0x1022
 #define AMD_SFH_HID_PRODUCT	0x0001
 
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
index 47774b9ab3de..cfda797f0a62 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.c
@@ -27,6 +27,7 @@ 
 #define ACEL_EN		BIT(0)
 #define GYRO_EN		BIT(1)
 #define MAGNO_EN	BIT(2)
+#define TMS_EN		BIT(15)
 #define HPD_EN		BIT(16)
 #define ALS_EN		BIT(19)
 
@@ -227,6 +228,9 @@  int amd_mp2_get_sensor_num(struct amd_mp2_dev *privdata, u8 *sensor_id)
 	if (MAGNO_EN & activestatus)
 		sensor_id[num_of_sensors++] = mag_idx;
 
+	if (TMS_EN & activestatus)
+		sensor_id[num_of_sensors++] = tms_idx;
+
 	if (ALS_EN & activestatus)
 		sensor_id[num_of_sensors++] = als_idx;
 
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h
index dfb7cabd82ef..e18ceee9e5db 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_pcie.h
@@ -78,6 +78,7 @@  enum sensor_idx {
 	accel_idx = 0,
 	gyro_idx = 1,
 	mag_idx = 2,
+	tms_idx = 15,
 	als_idx = 19
 };
 
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
index f9a8c02d5a7b..181973f35f05 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c
@@ -47,6 +47,11 @@  static int get_report_descriptor(int sensor_idx, u8 *rep_desc)
 		memcpy(rep_desc, comp3_report_descriptor,
 		       sizeof(comp3_report_descriptor));
 		break;
+	case tms_idx: /* tablet mode switch */
+		memset(rep_desc, 0, sizeof(tms_report_descriptor));
+		memcpy(rep_desc, tms_report_descriptor,
+		       sizeof(tms_report_descriptor));
+		break;
 	case als_idx: /* ambient light sensor */
 		memset(rep_desc, 0, sizeof(als_report_descriptor));
 		memcpy(rep_desc, als_report_descriptor,
@@ -96,6 +101,16 @@  static u32 get_descr_sz(int sensor_idx, int descriptor_name)
 			return sizeof(struct magno_feature_report);
 		}
 		break;
+	case tms_idx:
+		switch (descriptor_name) {
+		case descr_size:
+			return sizeof(tms_report_descriptor);
+		case input_size:
+			return sizeof(struct tms_input_report);
+		case feature_size:
+			return sizeof(struct tms_feature_report);
+		}
+		break;
 	case als_idx:
 		switch (descriptor_name) {
 		case descr_size:
@@ -138,6 +153,7 @@  static u8 get_feature_report(int sensor_idx, int report_id, u8 *feature_report)
 	struct accel3_feature_report acc_feature;
 	struct gyro_feature_report gyro_feature;
 	struct magno_feature_report magno_feature;
+	struct tms_feature_report tms_feature;
 	struct hpd_feature_report hpd_feature;
 	struct als_feature_report als_feature;
 	u8 report_size = 0;
@@ -173,6 +189,11 @@  static u8 get_feature_report(int sensor_idx, int report_id, u8 *feature_report)
 		memcpy(feature_report, &magno_feature, sizeof(magno_feature));
 		report_size = sizeof(magno_feature);
 		break;
+	case tms_idx:  /* tablet mode switch */
+		get_common_features(&tms_feature.common_property, report_id);
+		memcpy(feature_report, &tms_feature, sizeof(tms_feature));
+		report_size = sizeof(tms_feature);
+		break;
 	case als_idx:  /* ambient light sensor */
 		get_common_features(&als_feature.common_property, report_id);
 		als_feature.als_change_sesnitivity = HID_DEFAULT_SENSITIVITY;
@@ -211,6 +232,7 @@  static u8 get_input_report(u8 current_index, int sensor_idx, int report_id,
 	struct accel3_input_report acc_input;
 	struct gyro_input_report gyro_input;
 	struct hpd_input_report hpd_input;
+	struct tms_input_report tms_input;
 	struct als_input_report als_input;
 	struct hpd_status hpdstatus;
 	u8 report_size = 0;
@@ -244,6 +266,11 @@  static u8 get_input_report(u8 current_index, int sensor_idx, int report_id,
 		memcpy(input_report, &magno_input, sizeof(magno_input));
 		report_size = sizeof(magno_input);
 		break;
+	case tms_idx: /* tablet mode switch */
+		get_common_inputs(&tms_input.common_property, report_id);
+		report_size = sizeof(tms_input);
+		memcpy(input_report, &tms_input, sizeof(tms_input));
+		break;
 	case als_idx: /* Als */
 		get_common_inputs(&als_input.common_property, report_id);
 		/* For ALS ,V2 Platforms uses C2P_MSG5 register instead of DRAM access method */
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
index ebd55675eb62..b22068a47429 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h
@@ -111,4 +111,11 @@  struct hpd_input_report {
 	u8 human_presence;
 } __packed;
 
+struct tms_feature_report {
+	struct common_feature_property common_property;
+} __packed;
+
+struct tms_input_report {
+	struct common_input_property common_property;
+} __packed;
 #endif
diff --git a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
index 697f2791ea9c..96cbc1e5b9a7 100644
--- a/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
+++ b/drivers/hid/amd-sfh-hid/hid_descriptor/amd_sfh_hid_report_desc.h
@@ -644,6 +644,27 @@  static const u8 als_report_descriptor[] = {
 0xC0			/* HID end collection */
 };
 
+
+/* TABLET MODE SWITCH */
+__maybe_unused // Used by sfh1.0, but not yet implemented in sfh1.1
+static const u8 tms_report_descriptor[] = {
+0x06, 0x43, 0xFF,  // Usage Page (Vendor Defined 0xFF43)
+0x0A, 0x02, 0x02,  // Usage (0x0202)
+0xA1, 0x01, // Collection (Application)
+0x85, 0x11, //   Report ID (17)
+0x15, 0x00, //   Logical Minimum (0)
+0x25, 0x01, //   Logical Maximum (1)
+0x35, 0x00, //   Physical Minimum (0)
+0x45, 0x01, //   Physical Maximum (1)
+0x65, 0x00, //   Unit (None)
+0x55, 0x00, //   Unit Exponent (0)
+0x75, 0x01, //   Report Size (1)
+0x95, 0x98, //   Report Count (-104)
+0x81, 0x03, //   Input (Const,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
+0x91, 0x03, //   Output (Const,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
+0xC1, 0x00, // End Collection
+};
+
 /* BIOMETRIC PRESENCE*/
 static const u8 hpd_report_descriptor[] = {
 0x05, 0x20,          /* Usage page */