[PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Song Hongyan]

As accelerometer sensor becomes more and more popular, there are more
user scenarios have been developed, "Hinge" is a very important usecase
which needs two accelerometer sensors to calculate the included angle
of keyboard and screen.
In this case, two accelerometer sensors will be exposed. Currently,
IIO interface hasn't other way to distinguish two sensors with same
sensor type, except sensor name. So a new sensor name "accel_2nd_3d"
is added for secondary accelerometer sensor.

In HID level, connection type is a good common property to
differentiate two sensors with same sensor type.

Signed-off-by: Song Hongyan <hongyan.song-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/iio/accel/hid-sensor-accel-3d.c            | 35 ++++++++++++++++++++--
 .../iio/common/hid-sensors/hid-sensor-attributes.c |  5 ++++
 include/linux/hid-sensor-hub.h                     |  1 +
 include/linux/hid-sensor-ids.h                     |  1 +
 4 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index ca5759c..cc4366e 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -31,6 +31,13 @@
 #include <linux/iio/triggered_buffer.h>
 #include "../common/hid-sensors/hid-sensor-trigger.h"
 
+enum connection_type {
+	CONN_PC_INTEGRATED_SEL,
+	CONN_PC_ATTACHED_SEL,
+	CONN_PC_EXTERNAL_SEL,
+	CONN_TYPE_MAX,
+};
+
 enum accel_3d_channel {
 	CHANNEL_SCAN_INDEX_X,
 	CHANNEL_SCAN_INDEX_Y,
@@ -343,6 +350,19 @@ static int accel_3d_parse_report(struct platform_device *pdev,
 	return ret;
 }
 
+int connection_type_check(struct hid_sensor_common *st, int *conn_tp)
+{
+	int ret;
+
+	ret = sensor_hub_get_feature(st->hsdev, st->conn_type.report_id,
+		st->conn_type.index, sizeof(*conn_tp), conn_tp);
+
+	if (ret < 0 || conn_tp < 0)
+		return -EINVAL;
+
+	return ret;
+}
+
 /* Function to initialize the processing for usage id */
 static int hid_accel_3d_probe(struct platform_device *pdev)
 {
@@ -352,6 +372,7 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
 	struct accel_3d_state *accel_state;
 	const struct iio_chan_spec *channel_spec;
 	int channel_size;
+	s32 conn_type;
 
 	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
 
@@ -367,11 +388,9 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
 	accel_state->common_attributes.pdev = pdev;
 
 	if (hsdev->usage == HID_USAGE_SENSOR_ACCEL_3D) {
-		name = "accel_3d";
 		channel_spec = accel_3d_channels;
 		channel_size = sizeof(accel_3d_channels);
 	} else {
-		name = "gravity";
 		channel_spec = gravity_channels;
 		channel_size = sizeof(gravity_channels);
 	}
@@ -387,6 +406,18 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "failed to duplicate channels\n");
 		return -ENOMEM;
 	}
+
+	ret = connection_type_check(&accel_state->common_attributes,
+				&conn_type);
+
+	if (hsdev->usage == HID_USAGE_SENSOR_ACCEL_3D) {
+		if (conn_type == CONN_PC_EXTERNAL_SEL)
+			name = "accel_2nd_3d";
+		else
+			name = "accel_3d";
+	} else
+		name = "gravity";
+
 	ret = accel_3d_parse_report(pdev, hsdev,
 				(struct iio_chan_spec *)indio_dev->channels,
 				hsdev->usage, accel_state);
diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
index 4f280ae..cc2ce2a 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
@@ -400,6 +400,11 @@ int hid_sensor_parse_common_attributes(struct hid_sensor_hub_device *hsdev,
 			 &st->sensitivity);
 
 	sensor_hub_input_get_attribute_info(hsdev,
+					HID_FEATURE_REPORT, usage_id,
+					HID_USAGE_SENSOR_PROP_CONN_TYPE,
+					&st->conn_type);
+
+	sensor_hub_input_get_attribute_info(hsdev,
 					    HID_INPUT_REPORT, usage_id,
 					    HID_USAGE_SENSOR_TIME_TIMESTAMP,
 					    &timestamp);
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index 7ef111d..08756a9 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -237,6 +237,7 @@ struct hid_sensor_common {
 	struct hid_sensor_hub_attribute_info report_state;
 	struct hid_sensor_hub_attribute_info power_state;
 	struct hid_sensor_hub_attribute_info sensitivity;
+	struct hid_sensor_hub_attribute_info conn_type;
 	struct work_struct work;
 };
 
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 719c928..2d349f7 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -135,6 +135,7 @@
 #define HID_USAGE_SENSOR_UNITS_DEGREES_PER_SECOND		0x15
 
 /* Common selectors */
+#define HID_USAGE_SENSOR_PROP_CONN_TYPE				0x200309
 #define HID_USAGE_SENSOR_PROP_REPORT_INTERVAL			0x20030E
 #define HID_USAGE_SENSOR_PROP_SENSITIVITY_ABS			0x20030F
 #define HID_USAGE_SENSOR_PROP_SENSITIVITY_RANGE_PCT		0x200310
-- 
1.9.1

[PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Song Hongyan]

As accelerometer sensor becomes more and more popular, there are more
user scenarios have been developed, "Hinge" is a very important usecase
which needs two accelerometer sensors to calculate the included angle
of keyboard and screen.
In this case, two accelerometer sensors will be exposed. Currently,
IIO interface hasn't other way to distinguish two sensors with same
sensor type, except sensor name. So a new sensor name "accel_2nd_3d"
is added for secondary accelerometer sensor.

In HID level, connection type is a good common property to
differentiate two sensors with same sensor type.

Signed-off-by: Song Hongyan <hongyan.song@intel.com>
---
 drivers/iio/accel/hid-sensor-accel-3d.c            | 35 ++++++++++++++++++++--
 .../iio/common/hid-sensors/hid-sensor-attributes.c |  5 ++++
 include/linux/hid-sensor-hub.h                     |  1 +
 include/linux/hid-sensor-ids.h                     |  1 +
 4 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index ca5759c..cc4366e 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -31,6 +31,13 @@
 #include <linux/iio/triggered_buffer.h>
 #include "../common/hid-sensors/hid-sensor-trigger.h"
 
+enum connection_type {
+	CONN_PC_INTEGRATED_SEL,
+	CONN_PC_ATTACHED_SEL,
+	CONN_PC_EXTERNAL_SEL,
+	CONN_TYPE_MAX,
+};
+
 enum accel_3d_channel {
 	CHANNEL_SCAN_INDEX_X,
 	CHANNEL_SCAN_INDEX_Y,
@@ -343,6 +350,19 @@ static int accel_3d_parse_report(struct platform_device *pdev,
 	return ret;
 }
 
+int connection_type_check(struct hid_sensor_common *st, int *conn_tp)
+{
+	int ret;
+
+	ret = sensor_hub_get_feature(st->hsdev, st->conn_type.report_id,
+		st->conn_type.index, sizeof(*conn_tp), conn_tp);
+
+	if (ret < 0 || conn_tp < 0)
+		return -EINVAL;
+
+	return ret;
+}
+
 /* Function to initialize the processing for usage id */
 static int hid_accel_3d_probe(struct platform_device *pdev)
 {
@@ -352,6 +372,7 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
 	struct accel_3d_state *accel_state;
 	const struct iio_chan_spec *channel_spec;
 	int channel_size;
+	s32 conn_type;
 
 	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
 
@@ -367,11 +388,9 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
 	accel_state->common_attributes.pdev = pdev;
 
 	if (hsdev->usage == HID_USAGE_SENSOR_ACCEL_3D) {
-		name = "accel_3d";
 		channel_spec = accel_3d_channels;
 		channel_size = sizeof(accel_3d_channels);
 	} else {
-		name = "gravity";
 		channel_spec = gravity_channels;
 		channel_size = sizeof(gravity_channels);
 	}
@@ -387,6 +406,18 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "failed to duplicate channels\n");
 		return -ENOMEM;
 	}
+
+	ret = connection_type_check(&accel_state->common_attributes,
+				&conn_type);
+
+	if (hsdev->usage == HID_USAGE_SENSOR_ACCEL_3D) {
+		if (conn_type == CONN_PC_EXTERNAL_SEL)
+			name = "accel_2nd_3d";
+		else
+			name = "accel_3d";
+	} else
+		name = "gravity";
+
 	ret = accel_3d_parse_report(pdev, hsdev,
 				(struct iio_chan_spec *)indio_dev->channels,
 				hsdev->usage, accel_state);
diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
index 4f280ae..cc2ce2a 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
@@ -400,6 +400,11 @@ int hid_sensor_parse_common_attributes(struct hid_sensor_hub_device *hsdev,
 			 &st->sensitivity);
 
 	sensor_hub_input_get_attribute_info(hsdev,
+					HID_FEATURE_REPORT, usage_id,
+					HID_USAGE_SENSOR_PROP_CONN_TYPE,
+					&st->conn_type);
+
+	sensor_hub_input_get_attribute_info(hsdev,
 					    HID_INPUT_REPORT, usage_id,
 					    HID_USAGE_SENSOR_TIME_TIMESTAMP,
 					    &timestamp);
diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h
index 7ef111d..08756a9 100644
--- a/include/linux/hid-sensor-hub.h
+++ b/include/linux/hid-sensor-hub.h
@@ -237,6 +237,7 @@ struct hid_sensor_common {
 	struct hid_sensor_hub_attribute_info report_state;
 	struct hid_sensor_hub_attribute_info power_state;
 	struct hid_sensor_hub_attribute_info sensitivity;
+	struct hid_sensor_hub_attribute_info conn_type;
 	struct work_struct work;
 };
 
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index 719c928..2d349f7 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -135,6 +135,7 @@
 #define HID_USAGE_SENSOR_UNITS_DEGREES_PER_SECOND		0x15
 
 /* Common selectors */
+#define HID_USAGE_SENSOR_PROP_CONN_TYPE				0x200309
 #define HID_USAGE_SENSOR_PROP_REPORT_INTERVAL			0x20030E
 #define HID_USAGE_SENSOR_PROP_SENSITIVITY_ABS			0x20030F
 #define HID_USAGE_SENSOR_PROP_SENSITIVITY_RANGE_PCT		0x200310
-- 
1.9.1

Re: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Bastien Nocera]

On Thu, 2017-03-16 at 18:20 +0800, Song Hongyan wrote:
> As accelerometer sensor becomes more and more popular, there are more
> user scenarios have been developed, "Hinge" is a very important
> usecase
> which needs two accelerometer sensors to calculate the included angle
> of keyboard and screen.
> In this case, two accelerometer sensors will be exposed. Currently,
> IIO interface hasn't other way to distinguish two sensors with same
> sensor type, except sensor name. So a new sensor name "accel_2nd_3d"
> is added for secondary accelerometer sensor.
> 
> In HID level, connection type is a good common property to
> differentiate two sensors with same sensor type.

I've been told in the past not to rely on device names in iio-sensor-
proxy, and this would go against this advice.

As you have a "type" to export, why not export that instead?

RE: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Song, Hongyan]

Hi,
	Thanks for your comments. But I am not sure what your "type" mean.
If you mean "sensor type", I expose it as second ACC, the sensor type is the same with the exist ACC,
It cannot be used as a distinction.
If you mean the "connection type" I used in the patch, "connection type" is only defined in HID driver,
It is not a common property in IIO. So not good to expose it out.


BR
Song Hongyan  

-----Original Message-----
From: Bastien Nocera [mailto:hadess@hadess.net] 
Sent: Thursday, March 16, 2017 8:02 PM
To: Song, Hongyan <hongyan.song@intel.com>; linux-input@vger.kernel.org; linux-iio@vger.kernel.org
Cc: jikos@kernel.org; jic23@kernel.org; Pandruvada, Srinivas <srinivas.pandruvada@intel.com>
Subject: Re: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

On Thu, 2017-03-16 at 18:20 +0800, Song Hongyan wrote:
> As accelerometer sensor becomes more and more popular, there are more 
> user scenarios have been developed, "Hinge" is a very important 
> usecase which needs two accelerometer sensors to calculate the 
> included angle of keyboard and screen.
> In this case, two accelerometer sensors will be exposed. Currently, 
> IIO interface hasn't other way to distinguish two sensors with same 
> sensor type, except sensor name. So a new sensor name "accel_2nd_3d"
> is added for secondary accelerometer sensor.
> 
> In HID level, connection type is a good common property to 
> differentiate two sensors with same sensor type.

I've been told in the past not to rely on device names in iio-sensor- proxy, and this would go against this advice.

As you have a "type" to export, why not export that instead?

RE: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Song, Hongyan]

SGksDQoJVGhhbmtzIGZvciB5b3VyIGNvbW1lbnRzLiBCdXQgSSBhbSBub3Qgc3VyZSB3aGF0IHlv
dXIgInR5cGUiIG1lYW4uDQpJZiB5b3UgbWVhbiAic2Vuc29yIHR5cGUiLCBJIGV4cG9zZSBpdCBh
cyBzZWNvbmQgQUNDLCB0aGUgc2Vuc29yIHR5cGUgaXMgdGhlIHNhbWUgd2l0aCB0aGUgZXhpc3Qg
QUNDLA0KSXQgY2Fubm90IGJlIHVzZWQgYXMgYSBkaXN0aW5jdGlvbi4NCklmIHlvdSBtZWFuIHRo
ZSAiY29ubmVjdGlvbiB0eXBlIiBJIHVzZWQgaW4gdGhlIHBhdGNoLCAiY29ubmVjdGlvbiB0eXBl
IiBpcyBvbmx5IGRlZmluZWQgaW4gSElEIGRyaXZlciwNCkl0IGlzIG5vdCBhIGNvbW1vbiBwcm9w
ZXJ0eSBpbiBJSU8uIFNvIG5vdCBnb29kIHRvIGV4cG9zZSBpdCBvdXQuDQoNCg0KQlINClNvbmcg
SG9uZ3lhbiAgDQoNCi0tLS0tT3JpZ2luYWwgTWVzc2FnZS0tLS0tDQpGcm9tOiBCYXN0aWVuIE5v
Y2VyYSBbbWFpbHRvOmhhZGVzc0BoYWRlc3MubmV0XSANClNlbnQ6IFRodXJzZGF5LCBNYXJjaCAx
NiwgMjAxNyA4OjAyIFBNDQpUbzogU29uZywgSG9uZ3lhbiA8aG9uZ3lhbi5zb25nQGludGVsLmNv
bT47IGxpbnV4LWlucHV0QHZnZXIua2VybmVsLm9yZzsgbGludXgtaWlvQHZnZXIua2VybmVsLm9y
Zw0KQ2M6IGppa29zQGtlcm5lbC5vcmc7IGppYzIzQGtlcm5lbC5vcmc7IFBhbmRydXZhZGEsIFNy
aW5pdmFzIDxzcmluaXZhcy5wYW5kcnV2YWRhQGludGVsLmNvbT4NClN1YmplY3Q6IFJlOiBbUEFU
Q0hdIGlpbzogaGlkOiBoaWQtc2Vuc29yLWFjY2VsLTNkOiBBZGQgc2Vjb25kIEFDQyBzZW5zb3Ig
c3VwcG9ydA0KDQpPbiBUaHUsIDIwMTctMDMtMTYgYXQgMTg6MjAgKzA4MDAsIFNvbmcgSG9uZ3lh
biB3cm90ZToNCj4gQXMgYWNjZWxlcm9tZXRlciBzZW5zb3IgYmVjb21lcyBtb3JlIGFuZCBtb3Jl
IHBvcHVsYXIsIHRoZXJlIGFyZSBtb3JlIA0KPiB1c2VyIHNjZW5hcmlvcyBoYXZlIGJlZW4gZGV2
ZWxvcGVkLCAiSGluZ2UiIGlzIGEgdmVyeSBpbXBvcnRhbnQgDQo+IHVzZWNhc2Ugd2hpY2ggbmVl
ZHMgdHdvIGFjY2VsZXJvbWV0ZXIgc2Vuc29ycyB0byBjYWxjdWxhdGUgdGhlIA0KPiBpbmNsdWRl
ZCBhbmdsZSBvZiBrZXlib2FyZCBhbmQgc2NyZWVuLg0KPiBJbiB0aGlzIGNhc2UsIHR3byBhY2Nl
bGVyb21ldGVyIHNlbnNvcnMgd2lsbCBiZSBleHBvc2VkLiBDdXJyZW50bHksIA0KPiBJSU8gaW50
ZXJmYWNlIGhhc24ndCBvdGhlciB3YXkgdG8gZGlzdGluZ3Vpc2ggdHdvIHNlbnNvcnMgd2l0aCBz
YW1lIA0KPiBzZW5zb3IgdHlwZSwgZXhjZXB0IHNlbnNvciBuYW1lLiBTbyBhIG5ldyBzZW5zb3Ig
bmFtZSAiYWNjZWxfMm5kXzNkIg0KPiBpcyBhZGRlZCBmb3Igc2Vjb25kYXJ5IGFjY2VsZXJvbWV0
ZXIgc2Vuc29yLg0KPiANCj4gSW4gSElEIGxldmVsLCBjb25uZWN0aW9uIHR5cGUgaXMgYSBnb29k
IGNvbW1vbiBwcm9wZXJ0eSB0byANCj4gZGlmZmVyZW50aWF0ZSB0d28gc2Vuc29ycyB3aXRoIHNh
bWUgc2Vuc29yIHR5cGUuDQoNCkkndmUgYmVlbiB0b2xkIGluIHRoZSBwYXN0IG5vdCB0byByZWx5
IG9uIGRldmljZSBuYW1lcyBpbiBpaW8tc2Vuc29yLSBwcm94eSwgYW5kIHRoaXMgd291bGQgZ28g
YWdhaW5zdCB0aGlzIGFkdmljZS4NCg0KQXMgeW91IGhhdmUgYSAidHlwZSIgdG8gZXhwb3J0LCB3
aHkgbm90IGV4cG9ydCB0aGF0IGluc3RlYWQ/DQo=

Re: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Pandruvada, Srinivas]

On Thu, 2017-03-16 at 18:20 +0800, Song Hongyan wrote:
> As accelerometer sensor becomes more and more popular, there are more
> user scenarios have been developed, "Hinge" is a very important
> usecase
> which needs two accelerometer sensors to calculate the included angle
> of keyboard and screen.
> In this case, two accelerometer sensors will be exposed. Currently,
> IIO interface hasn't other way to distinguish two sensors with same
> sensor type, except sensor name. So a new sensor name "accel_2nd_3d"
> is added for secondary accelerometer sensor.

This type of interface will not satisfy all cases. We have some hubs
with  many accelerometers attached. Same case is also true even for
discrete sensors. So there should be some framework way to expose
location of sensors.

ACPI has special method called _PLD (Physical Device Location), which
can  be used to specify location of any device. So we need to be able
to export such information to user space. We can add for each sensor
the location information.

I can propose some ABI for exporting location information.

Thought?

Thanks,
Srinivas


> 
> In HID level, connection type is a good common property to
> differentiate two sensors with same sensor type.
> 
> Signed-off-by: Song Hongyan <hongyan.song@intel.com>
> ---
>  drivers/iio/accel/hid-sensor-accel-3d.c            | 35
> ++++++++++++++++++++--
>  .../iio/common/hid-sensors/hid-sensor-attributes.c |  5 ++++
>  include/linux/hid-sensor-hub.h                     |  1 +
>  include/linux/hid-sensor-ids.h                     |  1 +
>  4 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c
> b/drivers/iio/accel/hid-sensor-accel-3d.c
> index ca5759c..cc4366e 100644
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c
> @@ -31,6 +31,13 @@
>  #include <linux/iio/triggered_buffer.h>
>  #include "../common/hid-sensors/hid-sensor-trigger.h"
>  
> +enum connection_type {
> +	CONN_PC_INTEGRATED_SEL,
> +	CONN_PC_ATTACHED_SEL,
> +	CONN_PC_EXTERNAL_SEL,
> +	CONN_TYPE_MAX,
> +};
> +
>  enum accel_3d_channel {
>  	CHANNEL_SCAN_INDEX_X,
>  	CHANNEL_SCAN_INDEX_Y,
> @@ -343,6 +350,19 @@ static int accel_3d_parse_report(struct
> platform_device *pdev,
>  	return ret;
>  }
>  
> +int connection_type_check(struct hid_sensor_common *st, int
> *conn_tp)
> +{
> +	int ret;
> +
> +	ret = sensor_hub_get_feature(st->hsdev, st-
> >conn_type.report_id,
> +		st->conn_type.index, sizeof(*conn_tp), conn_tp);
> +
> +	if (ret < 0 || conn_tp < 0)
> +		return -EINVAL;
> +
> +	return ret;
> +}
> +
>  /* Function to initialize the processing for usage id */
>  static int hid_accel_3d_probe(struct platform_device *pdev)
>  {
> @@ -352,6 +372,7 @@ static int hid_accel_3d_probe(struct
> platform_device *pdev)
>  	struct accel_3d_state *accel_state;
>  	const struct iio_chan_spec *channel_spec;
>  	int channel_size;
> +	s32 conn_type;
>  
>  	struct hid_sensor_hub_device *hsdev = pdev-
> >dev.platform_data;
>  
> @@ -367,11 +388,9 @@ static int hid_accel_3d_probe(struct
> platform_device *pdev)
>  	accel_state->common_attributes.pdev = pdev;
>  
>  	if (hsdev->usage == HID_USAGE_SENSOR_ACCEL_3D) {
> -		name = "accel_3d";
>  		channel_spec = accel_3d_channels;
>  		channel_size = sizeof(accel_3d_channels);
>  	} else {
> -		name = "gravity";
>  		channel_spec = gravity_channels;
>  		channel_size = sizeof(gravity_channels);
>  	}
> @@ -387,6 +406,18 @@ static int hid_accel_3d_probe(struct
> platform_device *pdev)
>  		dev_err(&pdev->dev, "failed to duplicate
> channels\n");
>  		return -ENOMEM;
>  	}
> +
> +	ret = connection_type_check(&accel_state->common_attributes,
> +				&conn_type);
> +
> +	if (hsdev->usage == HID_USAGE_SENSOR_ACCEL_3D) {
> +		if (conn_type == CONN_PC_EXTERNAL_SEL)
> +			name = "accel_2nd_3d";
> +		else
> +			name = "accel_3d";
> +	} else
> +		name = "gravity";
> +
>  	ret = accel_3d_parse_report(pdev, hsdev,
>  				(struct iio_chan_spec *)indio_dev-
> >channels,
>  				hsdev->usage, accel_state);
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> index 4f280ae..cc2ce2a 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> @@ -400,6 +400,11 @@ int hid_sensor_parse_common_attributes(struct
> hid_sensor_hub_device *hsdev,
>  			 &st->sensitivity);
>  
>  	sensor_hub_input_get_attribute_info(hsdev,
> +					HID_FEATURE_REPORT,
> usage_id,
> +					HID_USAGE_SENSOR_PROP_CONN_T
> YPE,
> +					&st->conn_type);
> +
> +	sensor_hub_input_get_attribute_info(hsdev,
>  					    HID_INPUT_REPORT,
> usage_id,
>  					    HID_USAGE_SENSOR_TIME_TI
> MESTAMP,
>  					    &timestamp);
> diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-
> sensor-hub.h
> index 7ef111d..08756a9 100644
> --- a/include/linux/hid-sensor-hub.h
> +++ b/include/linux/hid-sensor-hub.h
> @@ -237,6 +237,7 @@ struct hid_sensor_common {
>  	struct hid_sensor_hub_attribute_info report_state;
>  	struct hid_sensor_hub_attribute_info power_state;
>  	struct hid_sensor_hub_attribute_info sensitivity;
> +	struct hid_sensor_hub_attribute_info conn_type;
>  	struct work_struct work;
>  };
>  
> diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-
> sensor-ids.h
> index 719c928..2d349f7 100644
> --- a/include/linux/hid-sensor-ids.h
> +++ b/include/linux/hid-sensor-ids.h
> @@ -135,6 +135,7 @@
>  #define HID_USAGE_SENSOR_UNITS_DEGREES_PER_SECOND		0x1
> 5
>  
>  /* Common selectors */
> +#define HID_USAGE_SENSOR_PROP_CONN_TYPE				
> 0x200309
>  #define HID_USAGE_SENSOR_PROP_REPORT_INTERVAL			
> 0x20030E
>  #define HID_USAGE_SENSOR_PROP_SENSITIVITY_ABS			
> 0x20030F
>  #define HID_USAGE_SENSOR_PROP_SENSITIVITY_RANGE_PCT		0
> x200310

Re: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Pandruvada, Srinivas]

T24gVGh1LCAyMDE3LTAzLTE2IGF0IDE4OjIwICswODAwLCBTb25nIEhvbmd5YW4gd3JvdGU6DQo+
IEFzIGFjY2VsZXJvbWV0ZXIgc2Vuc29yIGJlY29tZXMgbW9yZSBhbmQgbW9yZSBwb3B1bGFyLCB0
aGVyZSBhcmUgbW9yZQ0KPiB1c2VyIHNjZW5hcmlvcyBoYXZlIGJlZW4gZGV2ZWxvcGVkLCAiSGlu
Z2UiIGlzIGEgdmVyeSBpbXBvcnRhbnQNCj4gdXNlY2FzZQ0KPiB3aGljaCBuZWVkcyB0d28gYWNj
ZWxlcm9tZXRlciBzZW5zb3JzIHRvIGNhbGN1bGF0ZSB0aGUgaW5jbHVkZWQgYW5nbGUNCj4gb2Yg
a2V5Ym9hcmQgYW5kIHNjcmVlbi4NCj4gSW4gdGhpcyBjYXNlLCB0d28gYWNjZWxlcm9tZXRlciBz
ZW5zb3JzIHdpbGwgYmUgZXhwb3NlZC4gQ3VycmVudGx5LA0KPiBJSU8gaW50ZXJmYWNlIGhhc24n
dCBvdGhlciB3YXkgdG8gZGlzdGluZ3Vpc2ggdHdvIHNlbnNvcnMgd2l0aCBzYW1lDQo+IHNlbnNv
ciB0eXBlLCBleGNlcHQgc2Vuc29yIG5hbWUuIFNvIGEgbmV3IHNlbnNvciBuYW1lICJhY2NlbF8y
bmRfM2QiDQo+IGlzIGFkZGVkIGZvciBzZWNvbmRhcnkgYWNjZWxlcm9tZXRlciBzZW5zb3IuDQoN
ClRoaXMgdHlwZSBvZiBpbnRlcmZhY2Ugd2lsbCBub3Qgc2F0aXNmeSBhbGwgY2FzZXMuIFdlIGhh
dmUgc29tZSBodWJzDQp3aXRoICBtYW55IGFjY2VsZXJvbWV0ZXJzIGF0dGFjaGVkLiBTYW1lIGNh
c2UgaXMgYWxzbyB0cnVlIGV2ZW4gZm9yDQpkaXNjcmV0ZSBzZW5zb3JzLiBTbyB0aGVyZSBzaG91
bGQgYmUgc29tZSBmcmFtZXdvcmsgd2F5IHRvIGV4cG9zZQ0KbG9jYXRpb24gb2Ygc2Vuc29ycy4N
Cg0KQUNQSSBoYXMgc3BlY2lhbCBtZXRob2QgY2FsbGVkIF9QTEQgKFBoeXNpY2FsIERldmljZSBM
b2NhdGlvbiksIHdoaWNoDQpjYW4gIGJlIHVzZWQgdG8gc3BlY2lmeSBsb2NhdGlvbiBvZiBhbnkg
ZGV2aWNlLiBTbyB3ZSBuZWVkIHRvIGJlIGFibGUNCnRvIGV4cG9ydCBzdWNoIGluZm9ybWF0aW9u
IHRvIHVzZXIgc3BhY2UuIFdlIGNhbiBhZGQgZm9yIGVhY2ggc2Vuc29yDQp0aGUgbG9jYXRpb24g
aW5mb3JtYXRpb24uDQoNCkkgY2FuIHByb3Bvc2Ugc29tZSBBQkkgZm9yIGV4cG9ydGluZyBsb2Nh
dGlvbiBpbmZvcm1hdGlvbi4NCg0KVGhvdWdodD8NCg0KVGhhbmtzLA0KU3Jpbml2YXMNCg0KDQo+
IA0KPiBJbiBISUQgbGV2ZWwsIGNvbm5lY3Rpb24gdHlwZSBpcyBhIGdvb2QgY29tbW9uIHByb3Bl
cnR5IHRvDQo+IGRpZmZlcmVudGlhdGUgdHdvIHNlbnNvcnMgd2l0aCBzYW1lIHNlbnNvciB0eXBl
Lg0KPiANCj4gU2lnbmVkLW9mZi1ieTogU29uZyBIb25neWFuIDxob25neWFuLnNvbmdAaW50ZWwu
Y29tPg0KPiAtLS0NCj4gwqBkcml2ZXJzL2lpby9hY2NlbC9oaWQtc2Vuc29yLWFjY2VsLTNkLmPC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB8IDM1DQo+ICsrKysrKysrKysrKysrKysrKysrLS0NCj4g
wqAuLi4vaWlvL2NvbW1vbi9oaWQtc2Vuc29ycy9oaWQtc2Vuc29yLWF0dHJpYnV0ZXMuYyB8wqDC
oDUgKysrKw0KPiDCoGluY2x1ZGUvbGludXgvaGlkLXNlbnNvci1odWIuaMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoHzCoMKgMSArDQo+IMKgaW5jbHVkZS9saW51eC9o
aWQtc2Vuc29yLWlkcy5owqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
fMKgwqAxICsNCj4gwqA0IGZpbGVzIGNoYW5nZWQsIDQwIGluc2VydGlvbnMoKyksIDIgZGVsZXRp
b25zKC0pDQo+IA0KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9paW8vYWNjZWwvaGlkLXNlbnNvci1h
Y2NlbC0zZC5jDQo+IGIvZHJpdmVycy9paW8vYWNjZWwvaGlkLXNlbnNvci1hY2NlbC0zZC5jDQo+
IGluZGV4IGNhNTc1OWMuLmNjNDM2NmUgMTAwNjQ0DQo+IC0tLSBhL2RyaXZlcnMvaWlvL2FjY2Vs
L2hpZC1zZW5zb3ItYWNjZWwtM2QuYw0KPiArKysgYi9kcml2ZXJzL2lpby9hY2NlbC9oaWQtc2Vu
c29yLWFjY2VsLTNkLmMNCj4gQEAgLTMxLDYgKzMxLDEzIEBADQo+IMKgI2luY2x1ZGUgPGxpbnV4
L2lpby90cmlnZ2VyZWRfYnVmZmVyLmg+DQo+IMKgI2luY2x1ZGUgIi4uL2NvbW1vbi9oaWQtc2Vu
c29ycy9oaWQtc2Vuc29yLXRyaWdnZXIuaCINCj4gwqANCj4gK2VudW0gY29ubmVjdGlvbl90eXBl
IHsNCj4gKwlDT05OX1BDX0lOVEVHUkFURURfU0VMLA0KPiArCUNPTk5fUENfQVRUQUNIRURfU0VM
LA0KPiArCUNPTk5fUENfRVhURVJOQUxfU0VMLA0KPiArCUNPTk5fVFlQRV9NQVgsDQo+ICt9Ow0K
PiArDQo+IMKgZW51bSBhY2NlbF8zZF9jaGFubmVsIHsNCj4gwqAJQ0hBTk5FTF9TQ0FOX0lOREVY
X1gsDQo+IMKgCUNIQU5ORUxfU0NBTl9JTkRFWF9ZLA0KPiBAQCAtMzQzLDYgKzM1MCwxOSBAQCBz
dGF0aWMgaW50IGFjY2VsXzNkX3BhcnNlX3JlcG9ydChzdHJ1Y3QNCj4gcGxhdGZvcm1fZGV2aWNl
ICpwZGV2LA0KPiDCoAlyZXR1cm4gcmV0Ow0KPiDCoH0NCj4gwqANCj4gK2ludCBjb25uZWN0aW9u
X3R5cGVfY2hlY2soc3RydWN0IGhpZF9zZW5zb3JfY29tbW9uICpzdCwgaW50DQo+ICpjb25uX3Rw
KQ0KPiArew0KPiArCWludCByZXQ7DQo+ICsNCj4gKwlyZXQgPSBzZW5zb3JfaHViX2dldF9mZWF0
dXJlKHN0LT5oc2Rldiwgc3QtDQo+ID5jb25uX3R5cGUucmVwb3J0X2lkLA0KPiArCQlzdC0+Y29u
bl90eXBlLmluZGV4LCBzaXplb2YoKmNvbm5fdHApLCBjb25uX3RwKTsNCj4gKw0KPiArCWlmIChy
ZXQgPCAwIHx8IGNvbm5fdHAgPCAwKQ0KPiArCQlyZXR1cm4gLUVJTlZBTDsNCj4gKw0KPiArCXJl
dHVybiByZXQ7DQo+ICt9DQo+ICsNCj4gwqAvKiBGdW5jdGlvbiB0byBpbml0aWFsaXplIHRoZSBw
cm9jZXNzaW5nIGZvciB1c2FnZSBpZCAqLw0KPiDCoHN0YXRpYyBpbnQgaGlkX2FjY2VsXzNkX3By
b2JlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpDQo+IMKgew0KPiBAQCAtMzUyLDYgKzM3
Miw3IEBAIHN0YXRpYyBpbnQgaGlkX2FjY2VsXzNkX3Byb2JlKHN0cnVjdA0KPiBwbGF0Zm9ybV9k
ZXZpY2UgKnBkZXYpDQo+IMKgCXN0cnVjdCBhY2NlbF8zZF9zdGF0ZSAqYWNjZWxfc3RhdGU7DQo+
IMKgCWNvbnN0IHN0cnVjdCBpaW9fY2hhbl9zcGVjICpjaGFubmVsX3NwZWM7DQo+IMKgCWludCBj
aGFubmVsX3NpemU7DQo+ICsJczMyIGNvbm5fdHlwZTsNCj4gwqANCj4gwqAJc3RydWN0IGhpZF9z
ZW5zb3JfaHViX2RldmljZSAqaHNkZXYgPSBwZGV2LQ0KPiA+ZGV2LnBsYXRmb3JtX2RhdGE7DQo+
IMKgDQo+IEBAIC0zNjcsMTEgKzM4OCw5IEBAIHN0YXRpYyBpbnQgaGlkX2FjY2VsXzNkX3Byb2Jl
KHN0cnVjdA0KPiBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpDQo+IMKgCWFjY2VsX3N0YXRlLT5jb21t
b25fYXR0cmlidXRlcy5wZGV2ID0gcGRldjsNCj4gwqANCj4gwqAJaWYgKGhzZGV2LT51c2FnZSA9
PSBISURfVVNBR0VfU0VOU09SX0FDQ0VMXzNEKSB7DQo+IC0JCW5hbWUgPSAiYWNjZWxfM2QiOw0K
PiDCoAkJY2hhbm5lbF9zcGVjID0gYWNjZWxfM2RfY2hhbm5lbHM7DQo+IMKgCQljaGFubmVsX3Np
emUgPSBzaXplb2YoYWNjZWxfM2RfY2hhbm5lbHMpOw0KPiDCoAl9IGVsc2Ugew0KPiAtCQluYW1l
ID0gImdyYXZpdHkiOw0KPiDCoAkJY2hhbm5lbF9zcGVjID0gZ3Jhdml0eV9jaGFubmVsczsNCj4g
wqAJCWNoYW5uZWxfc2l6ZSA9IHNpemVvZihncmF2aXR5X2NoYW5uZWxzKTsNCj4gwqAJfQ0KPiBA
QCAtMzg3LDYgKzQwNiwxOCBAQCBzdGF0aWMgaW50IGhpZF9hY2NlbF8zZF9wcm9iZShzdHJ1Y3QN
Cj4gcGxhdGZvcm1fZGV2aWNlICpwZGV2KQ0KPiDCoAkJZGV2X2VycigmcGRldi0+ZGV2LCAiZmFp
bGVkIHRvIGR1cGxpY2F0ZQ0KPiBjaGFubmVsc1xuIik7DQo+IMKgCQlyZXR1cm4gLUVOT01FTTsN
Cj4gwqAJfQ0KPiArDQo+ICsJcmV0ID0gY29ubmVjdGlvbl90eXBlX2NoZWNrKCZhY2NlbF9zdGF0
ZS0+Y29tbW9uX2F0dHJpYnV0ZXMsDQo+ICsJCQkJJmNvbm5fdHlwZSk7DQo+ICsNCj4gKwlpZiAo
aHNkZXYtPnVzYWdlID09IEhJRF9VU0FHRV9TRU5TT1JfQUNDRUxfM0QpIHsNCj4gKwkJaWYgKGNv
bm5fdHlwZSA9PSBDT05OX1BDX0VYVEVSTkFMX1NFTCkNCj4gKwkJCW5hbWUgPSAiYWNjZWxfMm5k
XzNkIjsNCj4gKwkJZWxzZQ0KPiArCQkJbmFtZSA9ICJhY2NlbF8zZCI7DQo+ICsJfSBlbHNlDQo+
ICsJCW5hbWUgPSAiZ3Jhdml0eSI7DQo+ICsNCj4gwqAJcmV0ID0gYWNjZWxfM2RfcGFyc2VfcmVw
b3J0KHBkZXYsIGhzZGV2LA0KPiDCoAkJCQkoc3RydWN0IGlpb19jaGFuX3NwZWMgKilpbmRpb19k
ZXYtDQo+ID5jaGFubmVscywNCj4gwqAJCQkJaHNkZXYtPnVzYWdlLCBhY2NlbF9zdGF0ZSk7DQo+
IGRpZmYgLS1naXQgYS9kcml2ZXJzL2lpby9jb21tb24vaGlkLXNlbnNvcnMvaGlkLXNlbnNvci1h
dHRyaWJ1dGVzLmMNCj4gYi9kcml2ZXJzL2lpby9jb21tb24vaGlkLXNlbnNvcnMvaGlkLXNlbnNv
ci1hdHRyaWJ1dGVzLmMNCj4gaW5kZXggNGYyODBhZS4uY2MyY2UyYSAxMDA2NDQNCj4gLS0tIGEv
ZHJpdmVycy9paW8vY29tbW9uL2hpZC1zZW5zb3JzL2hpZC1zZW5zb3ItYXR0cmlidXRlcy5jDQo+
ICsrKyBiL2RyaXZlcnMvaWlvL2NvbW1vbi9oaWQtc2Vuc29ycy9oaWQtc2Vuc29yLWF0dHJpYnV0
ZXMuYw0KPiBAQCAtNDAwLDYgKzQwMCwxMSBAQCBpbnQgaGlkX3NlbnNvcl9wYXJzZV9jb21tb25f
YXR0cmlidXRlcyhzdHJ1Y3QNCj4gaGlkX3NlbnNvcl9odWJfZGV2aWNlICpoc2RldiwNCj4gwqAJ
CQnCoCZzdC0+c2Vuc2l0aXZpdHkpOw0KPiDCoA0KPiDCoAlzZW5zb3JfaHViX2lucHV0X2dldF9h
dHRyaWJ1dGVfaW5mbyhoc2RldiwNCj4gKwkJCQkJSElEX0ZFQVRVUkVfUkVQT1JULA0KPiB1c2Fn
ZV9pZCwNCj4gKwkJCQkJSElEX1VTQUdFX1NFTlNPUl9QUk9QX0NPTk5fVA0KPiBZUEUsDQo+ICsJ
CQkJCSZzdC0+Y29ubl90eXBlKTsNCj4gKw0KPiArCXNlbnNvcl9odWJfaW5wdXRfZ2V0X2F0dHJp
YnV0ZV9pbmZvKGhzZGV2LA0KPiDCoAkJCQkJwqDCoMKgwqBISURfSU5QVVRfUkVQT1JULA0KPiB1
c2FnZV9pZCwNCj4gwqAJCQkJCcKgwqDCoMKgSElEX1VTQUdFX1NFTlNPUl9USU1FX1RJDQo+IE1F
U1RBTVAsDQo+IMKgCQkJCQnCoMKgwqDCoCZ0aW1lc3RhbXApOw0KPiBkaWZmIC0tZ2l0IGEvaW5j
bHVkZS9saW51eC9oaWQtc2Vuc29yLWh1Yi5oIGIvaW5jbHVkZS9saW51eC9oaWQtDQo+IHNlbnNv
ci1odWIuaA0KPiBpbmRleCA3ZWYxMTFkLi4wODc1NmE5IDEwMDY0NA0KPiAtLS0gYS9pbmNsdWRl
L2xpbnV4L2hpZC1zZW5zb3ItaHViLmgNCj4gKysrIGIvaW5jbHVkZS9saW51eC9oaWQtc2Vuc29y
LWh1Yi5oDQo+IEBAIC0yMzcsNiArMjM3LDcgQEAgc3RydWN0IGhpZF9zZW5zb3JfY29tbW9uIHsN
Cj4gwqAJc3RydWN0IGhpZF9zZW5zb3JfaHViX2F0dHJpYnV0ZV9pbmZvIHJlcG9ydF9zdGF0ZTsN
Cj4gwqAJc3RydWN0IGhpZF9zZW5zb3JfaHViX2F0dHJpYnV0ZV9pbmZvIHBvd2VyX3N0YXRlOw0K
PiDCoAlzdHJ1Y3QgaGlkX3NlbnNvcl9odWJfYXR0cmlidXRlX2luZm8gc2Vuc2l0aXZpdHk7DQo+
ICsJc3RydWN0IGhpZF9zZW5zb3JfaHViX2F0dHJpYnV0ZV9pbmZvIGNvbm5fdHlwZTsNCj4gwqAJ
c3RydWN0IHdvcmtfc3RydWN0IHdvcms7DQo+IMKgfTsNCj4gwqANCj4gZGlmZiAtLWdpdCBhL2lu
Y2x1ZGUvbGludXgvaGlkLXNlbnNvci1pZHMuaCBiL2luY2x1ZGUvbGludXgvaGlkLQ0KPiBzZW5z
b3ItaWRzLmgNCj4gaW5kZXggNzE5YzkyOC4uMmQzNDlmNyAxMDA2NDQNCj4gLS0tIGEvaW5jbHVk
ZS9saW51eC9oaWQtc2Vuc29yLWlkcy5oDQo+ICsrKyBiL2luY2x1ZGUvbGludXgvaGlkLXNlbnNv
ci1pZHMuaA0KPiBAQCAtMTM1LDYgKzEzNSw3IEBADQo+IMKgI2RlZmluZSBISURfVVNBR0VfU0VO
U09SX1VOSVRTX0RFR1JFRVNfUEVSX1NFQ09ORAkJMHgxDQo+IDUNCj4gwqANCj4gwqAvKiBDb21t
b24gc2VsZWN0b3JzICovDQo+ICsjZGVmaW5lIEhJRF9VU0FHRV9TRU5TT1JfUFJPUF9DT05OX1RZ
UEUJCQkJDQo+IDB4MjAwMzA5DQo+IMKgI2RlZmluZSBISURfVVNBR0VfU0VOU09SX1BST1BfUkVQ
T1JUX0lOVEVSVkFMCQkJDQo+IDB4MjAwMzBFDQo+IMKgI2RlZmluZSBISURfVVNBR0VfU0VOU09S
X1BST1BfU0VOU0lUSVZJVFlfQUJTCQkJDQo+IDB4MjAwMzBGDQo+IMKgI2RlZmluZSBISURfVVNB
R0VfU0VOU09SX1BST1BfU0VOU0lUSVZJVFlfUkFOR0VfUENUCQkwDQo+IHgyMDAzMTA=

Re: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Bastien Nocera]

On Fri, 2017-03-17 at 01:21 +0000, Song, Hongyan wrote:
> Hi,
> 	Thanks for your comments. But I am not sure what your "type"
> mean.

I mean the connector type, which you use to differentiate the 2
accelerometers. You should export this data instead of changing the
name of the sensor.

> If you mean "sensor type", I expose it as second ACC, the sensor type
> is the same with the exist ACC,
> It cannot be used as a distinction.
> If you mean the "connection type" I used in the patch, "connection
> type" is only defined in HID driver,
> It is not a common property in IIO. So not good to expose it out.
> 
> 
> BR
> Song Hongyan  
> 
> -----Original Message-----
> From: Bastien Nocera [mailto:hadess@hadess.net] 
> Sent: Thursday, March 16, 2017 8:02 PM
> To: Song, Hongyan <hongyan.song@intel.com>; linux-input@vger.kernel.o
> rg; linux-iio@vger.kernel.org
> Cc: jikos@kernel.org; jic23@kernel.org; Pandruvada, Srinivas <sriniva
> s.pandruvada@intel.com>
> Subject: Re: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC
> sensor support
> 
> On Thu, 2017-03-16 at 18:20 +0800, Song Hongyan wrote:
> > As accelerometer sensor becomes more and more popular, there are
> > more 
> > user scenarios have been developed, "Hinge" is a very important 
> > usecase which needs two accelerometer sensors to calculate the 
> > included angle of keyboard and screen.
> > In this case, two accelerometer sensors will be exposed.
> > Currently, 
> > IIO interface hasn't other way to distinguish two sensors with
> > same 
> > sensor type, except sensor name. So a new sensor name
> > "accel_2nd_3d"
> > is added for secondary accelerometer sensor.
> > 
> > In HID level, connection type is a good common property to 
> > differentiate two sensors with same sensor type.
> 
> I've been told in the past not to rely on device names in iio-sensor-
> proxy, and this would go against this advice.
> 
> As you have a "type" to export, why not export that instead?
> NrybXǧv^)޺{.n+{zn)w*\x1fjg\x1eݢj/zޖ2ޙ&)ߡa\x7f\x1eGh\x0fj:+vw٥

Re: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Bastien Nocera]

On Fri, 2017-03-17 at 02:13 +0000, Pandruvada, Srinivas wrote:
> On Thu, 2017-03-16 at 18:20 +0800, Song Hongyan wrote:
> > As accelerometer sensor becomes more and more popular, there are
> > more
> > user scenarios have been developed, "Hinge" is a very important
> > usecase
> > which needs two accelerometer sensors to calculate the included
> > angle
> > of keyboard and screen.
> > In this case, two accelerometer sensors will be exposed. Currently,
> > IIO interface hasn't other way to distinguish two sensors with same
> > sensor type, except sensor name. So a new sensor name
> > "accel_2nd_3d"
> > is added for secondary accelerometer sensor.
> 
> This type of interface will not satisfy all cases. We have some hubs
> with  many accelerometers attached. Same case is also true even for
> discrete sensors. So there should be some framework way to expose
> location of sensors.

So you're nacking as well?

> ACPI has special method called _PLD (Physical Device Location), which
> can  be used to specify location of any device. So we need to be able
> to export such information to user space. We can add for each sensor
> the location information.
> 
> I can propose some ABI for exporting location information.

There were patches floating around to do this, but they were never
finished:
http://www.spinics.net/lists/linux-acpi/msg51540.html

Cheers

Re: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Jonathan Cameron]

On 17 March 2017 13:16:34 GMT+00:00, Bastien Nocera <hadess-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org> wrote:
>On Fri, 2017-03-17 at 02:13 +0000, Pandruvada, Srinivas wrote:
>> On Thu, 2017-03-16 at 18:20 +0800, Song Hongyan wrote:
>> > As accelerometer sensor becomes more and more popular, there are
>> > more
>> > user scenarios have been developed, "Hinge" is a very important
>> > usecase
>> > which needs two accelerometer sensors to calculate the included
>> > angle
>> > of keyboard and screen.
>> > In this case, two accelerometer sensors will be exposed. Currently,
>> > IIO interface hasn't other way to distinguish two sensors with same
>> > sensor type, except sensor name. So a new sensor name
>> > "accel_2nd_3d"
>> > is added for secondary accelerometer sensor.
>> 
>> This type of interface will not satisfy all cases. We have some hubs
>> with  many accelerometers attached. Same case is also true even for
>> discrete sensors. So there should be some framework way to expose
>> location of sensors.
>
>So you're nacking as well?
>
>> ACPI has special method called _PLD (Physical Device Location), which
>> can  be used to specify location of any device. So we need to be able
>> to export such information to user space. We can add for each sensor
>> the location information.
>> 
>> I can propose some ABI for exporting location information.
>
>There were patches floating around to do this, but they were never
>finished:
>http://www.spinics.net/lists/linux-acpi/msg51540.html

We have the location element as defined for the cross_ec sensors.  Taking that general would be fine with me.

 Current options are base and lid, but can be extended to include more.

J
>
>Cheers
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Jonathan Cameron]

On 17 March 2017 13:16:34 GMT+00:00, Bastien Nocera <hadess@hadess.net> wrote:
>On Fri, 2017-03-17 at 02:13 +0000, Pandruvada, Srinivas wrote:
>> On Thu, 2017-03-16 at 18:20 +0800, Song Hongyan wrote:
>> > As accelerometer sensor becomes more and more popular, there are
>> > more
>> > user scenarios have been developed, "Hinge" is a very important
>> > usecase
>> > which needs two accelerometer sensors to calculate the included
>> > angle
>> > of keyboard and screen.
>> > In this case, two accelerometer sensors will be exposed. Currently,
>> > IIO interface hasn't other way to distinguish two sensors with same
>> > sensor type, except sensor name. So a new sensor name
>> > "accel_2nd_3d"
>> > is added for secondary accelerometer sensor.
>> 
>> This type of interface will not satisfy all cases. We have some hubs
>> with  many accelerometers attached. Same case is also true even for
>> discrete sensors. So there should be some framework way to expose
>> location of sensors.
>
>So you're nacking as well?
>
>> ACPI has special method called _PLD (Physical Device Location), which
>> can  be used to specify location of any device. So we need to be able
>> to export such information to user space. We can add for each sensor
>> the location information.
>> 
>> I can propose some ABI for exporting location information.
>
>There were patches floating around to do this, but they were never
>finished:
>http://www.spinics.net/lists/linux-acpi/msg51540.html

We have the location element as defined for the cross_ec sensors.  Taking that general would be fine with me.

 Current options are base and lid, but can be extended to include more.

J
>
>Cheers
>--
>To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Pandruvada, Srinivas]

On Fri, 2017-03-17 at 14:16 +0100, Bastien Nocera wrote:
> On Fri, 2017-03-17 at 02:13 +0000, Pandruvada, Srinivas wrote:
> > 
> > On Thu, 2017-03-16 at 18:20 +0800, Song Hongyan wrote:
> > > 
> > > As accelerometer sensor becomes more and more popular, there are
> > > more
> > > user scenarios have been developed, "Hinge" is a very important
> > > usecase
> > > which needs two accelerometer sensors to calculate the included
> > > angle
> > > of keyboard and screen.
> > > In this case, two accelerometer sensors will be exposed.
> > > Currently,
> > > IIO interface hasn't other way to distinguish two sensors with
> > > same
> > > sensor type, except sensor name. So a new sensor name
> > > "accel_2nd_3d"
> > > is added for secondary accelerometer sensor.
> > 
> > This type of interface will not satisfy all cases. We have some
> > hubs
> > with  many accelerometers attached. Same case is also true even for
> > discrete sensors. So there should be some framework way to expose
> > location of sensors.
> 
> So you're nacking as well?
Yes.

> 
> > 
> > ACPI has special method called _PLD (Physical Device Location),
> > which
> > can  be used to specify location of any device. So we need to be
> > able
> > to export such information to user space. We can add for each
> > sensor
> > the location information.
> > 
> > I can propose some ABI for exporting location information.
> 
> There were patches floating around to do this, but they were never
> finished:
> http://www.spinics.net/lists/linux-acpi/msg51540.html
We need to add some of those element related to sensors to IIO ABI.
I will propose an ABI as Jonathan suggested.

Thanks,
Srinivas

Re: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Pandruvada, Srinivas]

T24gRnJpLCAyMDE3LTAzLTE3IGF0IDE0OjE2ICswMTAwLCBCYXN0aWVuIE5vY2VyYSB3cm90ZToN
Cj4gT24gRnJpLCAyMDE3LTAzLTE3IGF0IDAyOjEzICswMDAwLCBQYW5kcnV2YWRhLCBTcmluaXZh
cyB3cm90ZToNCj4gPiANCj4gPiBPbiBUaHUsIDIwMTctMDMtMTYgYXQgMTg6MjAgKzA4MDAsIFNv
bmcgSG9uZ3lhbiB3cm90ZToNCj4gPiA+IA0KPiA+ID4gQXMgYWNjZWxlcm9tZXRlciBzZW5zb3Ig
YmVjb21lcyBtb3JlIGFuZCBtb3JlIHBvcHVsYXIsIHRoZXJlIGFyZQ0KPiA+ID4gbW9yZQ0KPiA+
ID4gdXNlciBzY2VuYXJpb3MgaGF2ZSBiZWVuIGRldmVsb3BlZCwgIkhpbmdlIiBpcyBhIHZlcnkg
aW1wb3J0YW50DQo+ID4gPiB1c2VjYXNlDQo+ID4gPiB3aGljaCBuZWVkcyB0d28gYWNjZWxlcm9t
ZXRlciBzZW5zb3JzIHRvIGNhbGN1bGF0ZSB0aGUgaW5jbHVkZWQNCj4gPiA+IGFuZ2xlDQo+ID4g
PiBvZiBrZXlib2FyZCBhbmQgc2NyZWVuLg0KPiA+ID4gSW4gdGhpcyBjYXNlLCB0d28gYWNjZWxl
cm9tZXRlciBzZW5zb3JzIHdpbGwgYmUgZXhwb3NlZC4NCj4gPiA+IEN1cnJlbnRseSwNCj4gPiA+
IElJTyBpbnRlcmZhY2UgaGFzbid0IG90aGVyIHdheSB0byBkaXN0aW5ndWlzaCB0d28gc2Vuc29y
cyB3aXRoDQo+ID4gPiBzYW1lDQo+ID4gPiBzZW5zb3IgdHlwZSwgZXhjZXB0IHNlbnNvciBuYW1l
LiBTbyBhIG5ldyBzZW5zb3IgbmFtZQ0KPiA+ID4gImFjY2VsXzJuZF8zZCINCj4gPiA+IGlzIGFk
ZGVkIGZvciBzZWNvbmRhcnkgYWNjZWxlcm9tZXRlciBzZW5zb3IuDQo+ID4gDQo+ID4gVGhpcyB0
eXBlIG9mIGludGVyZmFjZSB3aWxsIG5vdCBzYXRpc2Z5IGFsbCBjYXNlcy4gV2UgaGF2ZSBzb21l
DQo+ID4gaHVicw0KPiA+IHdpdGjCoMKgbWFueSBhY2NlbGVyb21ldGVycyBhdHRhY2hlZC4gU2Ft
ZSBjYXNlIGlzIGFsc28gdHJ1ZSBldmVuIGZvcg0KPiA+IGRpc2NyZXRlIHNlbnNvcnMuIFNvIHRo
ZXJlIHNob3VsZCBiZSBzb21lIGZyYW1ld29yayB3YXkgdG8gZXhwb3NlDQo+ID4gbG9jYXRpb24g
b2Ygc2Vuc29ycy4NCj4gDQo+IFNvIHlvdSdyZSBuYWNraW5nIGFzIHdlbGw/DQpZZXMuDQoNCj4g
DQo+ID4gDQo+ID4gQUNQSSBoYXMgc3BlY2lhbCBtZXRob2QgY2FsbGVkIF9QTEQgKFBoeXNpY2Fs
IERldmljZSBMb2NhdGlvbiksDQo+ID4gd2hpY2gNCj4gPiBjYW7CoMKgYmUgdXNlZCB0byBzcGVj
aWZ5IGxvY2F0aW9uIG9mIGFueSBkZXZpY2UuIFNvIHdlIG5lZWQgdG8gYmUNCj4gPiBhYmxlDQo+
ID4gdG8gZXhwb3J0IHN1Y2ggaW5mb3JtYXRpb24gdG8gdXNlciBzcGFjZS4gV2UgY2FuIGFkZCBm
b3IgZWFjaA0KPiA+IHNlbnNvcg0KPiA+IHRoZSBsb2NhdGlvbiBpbmZvcm1hdGlvbi4NCj4gPiAN
Cj4gPiBJIGNhbiBwcm9wb3NlIHNvbWUgQUJJIGZvciBleHBvcnRpbmcgbG9jYXRpb24gaW5mb3Jt
YXRpb24uDQo+IA0KPiBUaGVyZSB3ZXJlIHBhdGNoZXMgZmxvYXRpbmcgYXJvdW5kIHRvIGRvIHRo
aXMsIGJ1dCB0aGV5IHdlcmUgbmV2ZXINCj4gZmluaXNoZWQ6DQo+IGh0dHA6Ly93d3cuc3Bpbmlj
cy5uZXQvbGlzdHMvbGludXgtYWNwaS9tc2c1MTU0MC5odG1sDQpXZSBuZWVkIHRvIGFkZCBzb21l
IG9mIHRob3NlIGVsZW1lbnQgcmVsYXRlZCB0byBzZW5zb3JzIHRvIElJTyBBQkkuDQpJIHdpbGwg
cHJvcG9zZSBhbiBBQkkgYXMgSm9uYXRoYW4gc3VnZ2VzdGVkLg0KDQpUaGFua3MsDQpTcmluaXZh
cw0K

Re: [PATCH] iio: hid: hid-sensor-accel-3d: Add second ACC sensor support

[Milton Mobley]

[-- Attachment #1: Type: text/plain, Size: 3898 bytes --]

Your device naming convention should be:

1) consistent with usual linux kernel practice
2) compatible with acpi-based systems (x86 and arm64)
3) compatible with device tree based systems (all others, I believe).

Here is some info from my dell 2-in-1 touch laptop:

Device(u'/sys/devices/pci0000:00/0000:00:15.0/i2c_designware.0/i2c-1/i2c-
DELL0742:00/0018:06CB:7E92.0009/input/input13')

According to the description in acpi programming reference, this suggests
acpi loaded the pci bus enumerator, which found the i2c-designware
host controllers, called the i2c_designwarer driver's probe function, which
did not scan the bus for slaves. But someone else has, possibly a driver
in the input system, and created the /sys node.

As you can see, '/', ':', '-' and '.' are being used as separators.

My hardware info says the laptop has two designware i2c host controllers,
thus the 'i2c-1' above refers to one of these.
I don't know what the following stuff is, but the '.0009' is probably an
i2c slave device specific value.

So you could, for example use 'accel_3d.1', etc for your naming. Where you
would create it is an open question.

Finally, I see some general problems in creating a system to monitor many
sensors:

Common sensors  don't have usb interfaces, so you couldn't use usb or hid
interface to control them.

These sensors do have either i2c or spi or both interfaces. Some have uart
interfaces

Sensors with i2c interfaces may have either one or two slave addresses. If
two, the address is selected by an input pin.
There is probably no name register you could program or use to distinguish
two devices on the same bus.
You would have to store the slave address somewhere, perhaps in a struct
with a name like 'chip_info'.

Of course in a device tree based system, you could just declare the slaves
as resources, and specify their slave addresses.

spi has the problem that 'in band' slave addressing is not supported, i.e.
there is no slave address in the data messages,
so you have to use extra gpio pins to select multiple devices.

On Fri, Mar 17, 2017 at 9:22 AM, Pandruvada, Srinivas <
srinivas.pandruvada@intel.com> wrote:

> On Fri, 2017-03-17 at 14:16 +0100, Bastien Nocera wrote:
> > On Fri, 2017-03-17 at 02:13 +0000, Pandruvada, Srinivas wrote:
> > >
> > > On Thu, 2017-03-16 at 18:20 +0800, Song Hongyan wrote:
> > > >
> > > > As accelerometer sensor becomes more and more popular, there are
> > > > more
> > > > user scenarios have been developed, "Hinge" is a very important
> > > > usecase
> > > > which needs two accelerometer sensors to calculate the included
> > > > angle
> > > > of keyboard and screen.
> > > > In this case, two accelerometer sensors will be exposed.
> > > > Currently,
> > > > IIO interface hasn't other way to distinguish two sensors with
> > > > same
> > > > sensor type, except sensor name. So a new sensor name
> > > > "accel_2nd_3d"
> > > > is added for secondary accelerometer sensor.
> > >
> > > This type of interface will not satisfy all cases. We have some
> > > hubs
> > > with  many accelerometers attached. Same case is also true even for
> > > discrete sensors. So there should be some framework way to expose
> > > location of sensors.
> >
> > So you're nacking as well?
> Yes.
>
> >
> > >
> > > ACPI has special method called _PLD (Physical Device Location),
> > > which
> > > can  be used to specify location of any device. So we need to be
> > > able
> > > to export such information to user space. We can add for each
> > > sensor
> > > the location information.
> > >
> > > I can propose some ABI for exporting location information.
> >
> > There were patches floating around to do this, but they were never
> > finished:
> > http://www.spinics.net/lists/linux-acpi/msg51540.html
> We need to add some of those element related to sensors to IIO ABI.
> I will propose an ABI as Jonathan suggested.
>
> Thanks,
> Srinivas
>

[-- Attachment #2: Type: text/html, Size: 5304 bytes --]