2022-12-14 21:54:09

by Adrian Freund

[permalink] [raw]
Subject: [PATCH] HID: amd_sfh: Add support for tablet-mode-switch sensor

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 <[email protected]>
Signed-off-by: Ivan Dovgal <[email protected]>
Co-developed-by: Luke D. Jones <[email protected]>
Signed-off-by: Luke D. Jones <[email protected]>
Signed-off-by: Adrian Freund <[email protected]>
---
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 | 20 ++++++++++++++
7 files changed, 62 insertions(+), 1 deletion(-)

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..a05e65d4db4c 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,26 @@ static const u8 als_report_descriptor[] = {
0xC0 /* HID end collection */
};

+
+/* TABLET MODE SWITCH */
+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 */
--
2.38.1


2022-12-15 08:34:13

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] HID: amd_sfh: Add support for tablet-mode-switch sensor

Hi Adrian,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on hid/for-next]
[also build test WARNING on linus/master v6.1 next-20221215]
[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/Adrian-Freund/HID-amd_sfh-Add-support-for-tablet-mode-switch-sensor/20221215-054325
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20221214214127.15347-1-adrian%40freund.io
patch subject: [PATCH] HID: amd_sfh: Add support for tablet-mode-switch sensor
config: x86_64-allyesconfig
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce (this is a W=1 build):
# https://github.com/intel-lab-lkp/linux/commit/9523955771c5517417b71bdcb1a19d8fadbc946d
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Adrian-Freund/HID-amd_sfh-Add-support-for-tablet-mode-switch-sensor/20221215-054325
git checkout 9523955771c5517417b71bdcb1a19d8fadbc946d
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/hid/amd-sfh-hid/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

In file included from drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c:15:
>> drivers/hid/amd-sfh-hid/sfh1_1/../hid_descriptor/amd_sfh_hid_report_desc.h:649:17: warning: 'tms_report_descriptor' defined but not used [-Wunused-const-variable=]
649 | static const u8 tms_report_descriptor[] = {
| ^~~~~~~~~~~~~~~~~~~~~


vim +/tms_report_descriptor +649 drivers/hid/amd-sfh-hid/sfh1_1/../hid_descriptor/amd_sfh_hid_report_desc.h

646
647
648 /* TABLET MODE SWITCH */
> 649 static const u8 tms_report_descriptor[] = {
650 0x06, 0x43, 0xFF, // Usage Page (Vendor Defined 0xFF43)
651 0x0A, 0x02, 0x02, // Usage (0x0202)
652 0xA1, 0x01, // Collection (Application)
653 0x85, 0x11, // Report ID (17)
654 0x15, 0x00, // Logical Minimum (0)
655 0x25, 0x01, // Logical Maximum (1)
656 0x35, 0x00, // Physical Minimum (0)
657 0x45, 0x01, // Physical Maximum (1)
658 0x65, 0x00, // Unit (None)
659 0x55, 0x00, // Unit Exponent (0)
660 0x75, 0x01, // Report Size (1)
661 0x95, 0x98, // Report Count (-104)
662 0x81, 0x03, // Input (Const,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
663 0x91, 0x03, // Output (Const,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
664 0xC1, 0x00, // End Collection
665 };
666

--
0-DAY CI Kernel Test Service
https://01.org/lkp


Attachments:
(No filename) (2.94 kB)
config (297.65 kB)
Download all attachments

2022-12-15 10:52:14

by Adrian Freund

[permalink] [raw]
Subject: Re: [PATCH] HID: amd_sfh: Add support for tablet-mode-switch sensor

On 12/15/22 09:22, kernel test robot wrote:
> Hi Adrian,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on hid/for-next]
> [also build test WARNING on linus/master v6.1 next-20221215]
> [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/Adrian-Freund/HID-amd_sfh-Add-support-for-tablet-mode-switch-sensor/20221215-054325
> base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
> patch link: https://lore.kernel.org/r/20221214214127.15347-1-adrian%40freund.io
> patch subject: [PATCH] HID: amd_sfh: Add support for tablet-mode-switch sensor
> config: x86_64-allyesconfig
> compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
> reproduce (this is a W=1 build):
> # https://github.com/intel-lab-lkp/linux/commit/9523955771c5517417b71bdcb1a19d8fadbc946d
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Adrian-Freund/HID-amd_sfh-Add-support-for-tablet-mode-switch-sensor/20221215-054325
> git checkout 9523955771c5517417b71bdcb1a19d8fadbc946d
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/hid/amd-sfh-hid/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> In file included from drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c:15:
>>> drivers/hid/amd-sfh-hid/sfh1_1/../hid_descriptor/amd_sfh_hid_report_desc.h:649:17: warning: 'tms_report_descriptor' defined but not used [-Wunused-const-variable=]
> 649 | static const u8 tms_report_descriptor[] = {
> | ^~~~~~~~~~~~~~~~~~~~~
hid_descriptor/amd_sfh_hid_report_desc.h is included from both
hid_descriptor/amd_sfh_hid_desc.c and sfh1_1/amd_sfh_desc.c, the first
of which has 4 usages of tms_report_descriptor. The later is for sensor
fusion hub 1.1. I don't have access to a devices using sfh1.1, so I
can't add support for the tablet mode switch there, causing the variable
to be unused for that import.
>
> vim +/tms_report_descriptor +649 drivers/hid/amd-sfh-hid/sfh1_1/../hid_descriptor/amd_sfh_hid_report_desc.h
>
> 646
> 647
> 648 /* TABLET MODE SWITCH */
> > 649 static const u8 tms_report_descriptor[] = {
> 650 0x06, 0x43, 0xFF, // Usage Page (Vendor Defined 0xFF43)
> 651 0x0A, 0x02, 0x02, // Usage (0x0202)
> 652 0xA1, 0x01, // Collection (Application)
> 653 0x85, 0x11, // Report ID (17)
> 654 0x15, 0x00, // Logical Minimum (0)
> 655 0x25, 0x01, // Logical Maximum (1)
> 656 0x35, 0x00, // Physical Minimum (0)
> 657 0x45, 0x01, // Physical Maximum (1)
> 658 0x65, 0x00, // Unit (None)
> 659 0x55, 0x00, // Unit Exponent (0)
> 660 0x75, 0x01, // Report Size (1)
> 661 0x95, 0x98, // Report Count (-104)
> 662 0x81, 0x03, // Input (Const,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
> 663 0x91, 0x03, // Output (Const,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile)
> 664 0xC1, 0x00, // End Collection
> 665 };
> 666
>
Adrian

2022-12-15 11:26:06

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH] HID: amd_sfh: Add support for tablet-mode-switch sensor

On Thu, 15 Dec 2022, Adrian Freund wrote:

> > In file included from drivers/hid/amd-sfh-hid/sfh1_1/amd_sfh_desc.c:15:
> >>> drivers/hid/amd-sfh-hid/sfh1_1/../hid_descriptor/amd_sfh_hid_report_desc.h:649:17:
> >>> warning: 'tms_report_descriptor' defined but not used
> >>> [-Wunused-const-variable=]
> > 649 | static const u8 tms_report_descriptor[] = {
> > | ^~~~~~~~~~~~~~~~~~~~~
> hid_descriptor/amd_sfh_hid_report_desc.h is included from both
> hid_descriptor/amd_sfh_hid_desc.c and sfh1_1/amd_sfh_desc.c, the first of
> which has 4 usages of tms_report_descriptor. The later is for sensor fusion
> hub 1.1. I don't have access to a devices using sfh1.1, so I can't add support
> for the tablet mode switch there, causing the variable to be unused for that
> import.

I'd say either move the rdesc directly to
hid_descriptor/amd_sfh_hid_desc.c, or alternatively mark it with
__attribute__((used)).

--
Jiri Kosina
SUSE Labs