Re: [PATCH 1/4] HID: hid-sensor-custom: Add custom sensor iio support
Hi Ye,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on iio/togreg]
[also build test WARNING on linus/master v5.10-rc4 next-20201118]
[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]
url:
https://github.com/0day-ci/linux/commits/Ye-Xiang/add-custom-hinge-sensor-support/20201119-110842
base: https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
config: c6x-randconfig-r003-20201119 (attached as .config)
compiler: c6x-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/1f33733d65038ade4af057df7e5c126485ecb7c6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Ye-Xiang/add-custom-hinge-sensor-support/20201119-110842
git checkout 1f33733d65038ade4af057df7e5c126485ecb7c6
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=c6x
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
drivers/hid/hid-sensor-custom.c: In function 'store_value':
drivers/hid/hid-sensor-custom.c:401:7: warning: variable 'ret' set but not
used [-Wunused-but-set-variable]
401 | int ret;
| ^~~
In file included from include/linux/device.h:15,
from include/linux/miscdevice.h:7,
from drivers/hid/hid-sensor-custom.c:11:
drivers/hid/hid-sensor-custom.c: In function 'get_known_custom_sensor_index':
>> drivers/hid/hid-sensor-custom.c:847:24: warning: format '%ld' expects
>> argument of type 'long int', but argument 3 has type '__kernel_size_t' {aka
>> 'unsigned int'} [-Wformat=]
847 | hid_err(hsdev->hdev, " %ld != %ld\n", strlen(buf),
|^~~
include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
19 | #define dev_fmt(fmt) fmt
| ^~~
include/linux/hid.h:1180:2: note: in expansion of macro 'dev_err'
1180 | dev_err(&(hid)->dev, fmt, ##__VA_ARGS__)
| ^~~
drivers/hid/hid-sensor-custom.c:847:3: note: in expansion of macro 'hid_err'
847 | hid_err(hsdev->hdev, " %ld != %ld\n", strlen(buf),
| ^~~
drivers/hid/hid-sensor-custom.c:847:28: note: format string is defined here
847 | hid_err(hsdev->hdev, " %ld != %ld\n", strlen(buf),
| ~~^
||
|long int
| %d
In file included from include/linux/device.h:15,
from include/linux/miscdevice.h:7,
from drivers/hid/hid-sensor-custom.c:11:
drivers/hid/hid-sensor-custom.c:847:24: warning: format '%ld' expects
argument of type 'long int', but argument 4 has type '__kernel_size_t' {aka
'unsigned int'} [-Wformat=]
847 | hid_err(hsdev->hdev, " %ld != %ld\n", strlen(buf),
|^~~
include/linux/dev_printk.h:19:22: note: in definition of macro 'dev_fmt'
19 | #define dev_fmt(fmt) fmt
| ^~~
include/linux/hid.h:1180:2: note: in expansion of macro 'dev_err'
1180 | dev_err(&(hid)->dev, fmt, ##__VA_ARGS__)
| ^~~
drivers/hid/hid-sensor-custom.c:847:3: note: in expansion of macro 'hid_err'
847 | hid_err(hsdev->hdev, " %ld != %ld\n", strlen(buf),
| ^~~
drivers/hid/hid-sensor-custom.c:847:35: note: format string is defined here
847 | hid_err(hsdev->hdev, " %ld != %ld\n", strlen(buf),
| ~~^
| |
| long int
| %d
vim +847 drivers/hid/hid-sensor-custom.c
771
772 static int get_known_custom_sensor_index(struct hid_sensor_hub_device
*hsdev)
773 {
774 struct hid_sensor_hub_attribute_info sensor_manufacturer = { 0
};
775 struct hid_sensor_hub_attribute_info sensor_luid_info = { 0 };
776 int report_size;
777 int ret;
778 u16 *w_buf;
779 int w_buf_len;
780 char *buf;
781 int buf_len;
782 int i;
783 int index;
784
785 w_buf_len = sizeof(u16) * HID_CUSTOM_MAX_FEATURE_BYTES;
786 buf_len = sizeof(char) * HID_CUSTOM_MAX_FEATURE_BYTES;
787 w_buf = kzalloc(w_buf_len, GFP_KERNEL);
788 if (!w_buf)
789 return -1;
[PATCH 1/4] HID: hid-sensor-custom: Add custom sensor iio support
Currently custom sensors properties are not decoded and it is up to
user space to interpret.
Some manufacturers already standardized the meaning of some custom sensors.
They can be presented as a proper IIO sensor. We can identify these sensors
based on manufacturer and serial number property in the report.
This change is identifying hinge sensor when the manufacturer is "INTEL".
This creates a platform device so that a sensor driver can be loaded to
process these sensors.
Signed-off-by: Ye Xiang
---
drivers/hid/hid-sensor-custom.c | 169
include/linux/hid-sensor-ids.h | 39
2 files changed, 208 insertions(+)
diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c
index 4d25577a8573..6ef8387405b3 100644
--- a/drivers/hid/hid-sensor-custom.c
+++ b/drivers/hid/hid-sensor-custom.c
@@ -4,6 +4,7 @@
* Copyright (c) 2015, Intel Corporation.
*/
+#include
#include
#include
#include
@@ -21,6 +22,7 @@
#define HID_CUSTOM_TOTAL_ATTRS (HID_CUSTOM_MAX_CORE_ATTRS + 1)
#define HID_CUSTOM_FIFO_SIZE 4096
#define HID_CUSTOM_MAX_FEATURE_BYTES 64
+#define HID_SENSOR_USAGE_LENGTH (4 + 1)
struct hid_sensor_custom_field {
int report_id;
@@ -50,6 +52,8 @@ struct hid_sensor_custom {
struct kfifo data_fifo;
unsigned long misc_opened;
wait_queue_head_t wait;
+ struct platform_device *custom_pdev;
+ bool custom_pdev_exposed;
};
/* Header for each sample to user space via dev interface */
@@ -746,11 +750,158 @@ static void hid_sensor_custom_dev_if_remove(struct
hid_sensor_custom
}
+/*
+ * use sensors luid which is defined in FW, such as ISH,
+ * to identify sensor.
+ */
+static char *known_sensor_luid[] = { "020B" };
+
+static int get_luid_table_index(unsigned char *usage_str)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(known_sensor_luid); i++) {
+ if (!strncmp(usage_str, known_sensor_luid[i],
+strlen(known_sensor_luid[i])))
+ return i;
+ }
+
+ return -1;
+}
+
+static int get_known_custom_sensor_index(struct hid_sensor_hub_device *hsdev)
+{
+ struct hid_sensor_hub_attribute_info sensor_manufacturer = { 0 };
+ struct hid_sensor_hub_attribute_info sensor_luid_info = { 0 };
+ int report_size;
+ int ret;
+ u16 *w_buf;
+ int w_buf_len;
+ char *buf;
+ int buf_len;
+ int i;
+ int index;
+
+ w_buf_len = sizeof(u16) * HID_CUSTOM_MAX_FEATURE_BYTES;
+ buf_len = sizeof(char) * HID_CUSTOM_MAX_FEATURE_BYTES;
+ w_buf = kzalloc(w_buf_len, GFP_KERNEL);
+ if (!w_buf)
+ return -1;
+
+ buf = kzalloc(buf_len, GFP_KERNEL);
+ if (!buf) {
+ kfree(w_buf);
+ return -1;
+ }
+
+ /* get manufacturer info */
+ ret = sensor_hub_input_get_attribute_info(
+ hsdev, HID_FEATURE_REPORT, hsdev->usage,
+ HID_USAGE_SENSOR_PROP_MANUFACTURER, _manufacturer);
+ if (ret < 0)
+ goto err_out;
+
+ report_size =
+ sensor_hub_get_feature(hsdev, sensor_manufacturer.report_id,
+ sensor_manufacturer.index, w_buf_len,
+ w_buf);
+ if (report_size <= 0) {
+ hid_err(hsdev->hdev,
+ "Failed to get sensor manufacturer info %d\n",
+ report_size);
+ goto err_out;
+ }
+
+ /* convert from wide char to char */
+ for (i = 0; i < buf_len - 1 && w_buf[i]; i++)
+ buf[i] = (char)w_buf[i];
+
+ /* ensure it's ISH sensor */
+ if (strncmp(buf, "INTEL", strlen("INTEL")))
+ goto err_out;
+
+ memset(w_buf, 0, w_buf_len);
+ memset(buf, 0, buf_len);
+
+ /* get real usage id */
+ ret = sensor_hub_input_get_attribute_info(
+ hsdev, HID_FEATURE_REPORT, hsdev->usage,
+ HID_USAGE_SENSOR_PROP_SERIAL_NUM, _luid_info);
+ if (ret < 0)
+ goto err_out;
+
+ report_size = sensor_hub_get_feature(hsdev, sensor_luid_info.report_id,
+sensor_luid_info.index, w_buf_len,
+w_buf);
+ if (report_size <= 0) {
+ hid_err(hsdev->hdev, "Failed to get real usage info %d\n",
+ report_size);
+ goto err_out;
+ }
+
+ /* convert from wide char to char */
+ for (i = 0; i < buf_len - 1 && w_buf[i]; i++)
+ buf[i] = (char)w_buf[i];
+
+ if (strlen(buf) != strlen(known_sensor_luid[0]) + 5) {
+ hid_err(hsdev->hdev, " %ld != %ld\n", strlen(buf),
+ strlen(known_sensor_luid[0]));
+ goto err_out;
+ }
+
+ /* get table index with luid (not matching 'LUID:
