All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] iio: hid: Add temperature sensor support
@ 2017-02-13  9:49 Song Hongyan
       [not found] ` <1486979375-44684-1-git-send-email-hongyan.song-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Song Hongyan @ 2017-02-13  9:49 UTC (permalink / raw)
  To: linux-input, linux-iio; +Cc: jikos, jic23, srinivas.pandruvada, Song Hongyan

Environmental temperature sensor is a hid defined sensor,
it measures temperature.

More information can be found in:
http://www.usb.org/developers/hidpage/HUTRR39b.pdf

According to IIO ABI definition, IIO_TEMP data output unit is
milli degrees Celsius. Add the unit convert from degree to milli degree.

Signed-off-by: Song Hongyan <hongyan.song@intel.com>
---
 .../iio/common/hid-sensors/hid-sensor-attributes.c |   3 +
 drivers/iio/temperature/Kconfig                    |  14 +
 drivers/iio/temperature/Makefile                   |   1 +
 drivers/iio/temperature/hid-sensor-temperature.c   | 326 +++++++++++++++++++++
 include/linux/hid-sensor-ids.h                     |   4 +
 5 files changed, 348 insertions(+)
 create mode 100644 drivers/iio/temperature/hid-sensor-temperature.c

diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
index 7ef94a9..68c2bb0 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
@@ -58,6 +58,9 @@
 
 	{HID_USAGE_SENSOR_PRESSURE, 0, 100, 0},
 	{HID_USAGE_SENSOR_PRESSURE, HID_USAGE_SENSOR_UNITS_PASCAL, 0, 1000000},
+
+	{HID_USAGE_SENSOR_TEMPERATURE, 0, 1000, 0},
+	{HID_USAGE_SENSOR_TEMPERATURE, HID_USAGE_SENSOR_UNITS_DEGREES, 1000, 0},
 };
 
 static int pow_10(unsigned power)
diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 5ea77a7..5f7b9f7 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -19,6 +19,20 @@ config MAXIM_THERMOCOUPLE
 	  This driver can also be built as a module. If so, the module will
 	  be called maxim_thermocouple.
 
+config HID_SENSOR_TEMP
+	tristate "HID Environmental temperature sensor"
+	depends on HID_SENSOR_HUB
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	select HID_SENSOR_IIO_COMMON
+	select HID_SENSOR_IIO_TRIGGER
+	help
+	  Say yes here to build support for the HID SENSOR
+	  temperature driver
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called hid-sensor-temperature.
+
 config MLX90614
 	tristate "MLX90614 contact-less infrared sensor"
 	depends on I2C
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index 78c3de0..e157eda 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_MLX90614) += mlx90614.o
 obj-$(CONFIG_TMP006) += tmp006.o
 obj-$(CONFIG_TSYS01) += tsys01.o
 obj-$(CONFIG_TSYS02D) += tsys02d.o
+obj-$(CONFIG_HID_SENSOR_TEMP)   += hid-sensor-temperature.o
diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
new file mode 100644
index 0000000..84abd2d
--- /dev/null
+++ b/drivers/iio/temperature/hid-sensor-temperature.c
@@ -0,0 +1,326 @@
+/* HID Sensors Driver
+ * Copyright (c) 2017, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.
+ *
+ */
+#include <linux/device.h>
+#include <linux/hid-sensor-hub.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include "../common/hid-sensors/hid-sensor-trigger.h"
+
+struct temperature_state {
+	struct hid_sensor_common common_attributes;
+	struct hid_sensor_hub_attribute_info temperature_attr;
+	s32 temperature_data;
+	int scale_pre_decml;
+	int scale_post_decml;
+	int scale_precision;
+	int value_offset;
+};
+
+/* Channel definitions */
+static const struct iio_chan_spec temperature_channels[] = {
+	{
+		.type = IIO_TEMP,
+		.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),
+		.scan_index = 0,
+	}
+};
+
+/* Adjust channel real bits based on report descriptor */
+static void temperature_adjust_channel_bit_mask(struct iio_chan_spec *channels,
+					int channel, int size)
+{
+	channels[channel].scan_type.sign = 's';
+	/* Real storage bits will change based on the report desc. */
+	channels[channel].scan_type.realbits = size * 8;
+	/* Maximum size of a sample to capture is s32 */
+	channels[channel].scan_type.storagebits = sizeof(s32) * 8;
+}
+
+/* Channel read_raw handler */
+static int temperature_read_raw(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				int *val, int *val2, long mask)
+{
+	struct temperature_state *temperature_state = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (chan->type == IIO_TEMP) {
+			hid_sensor_power_state(
+				&temperature_state->common_attributes, true);
+			*val = sensor_hub_input_attr_get_raw_value(
+				temperature_state->common_attributes.hsdev,
+				HID_USAGE_SENSOR_TEMPERATURE,
+				HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
+				temperature_state->temperature_attr.report_id,
+				SENSOR_HUB_SYNC);
+			hid_sensor_power_state(
+					&temperature_state->common_attributes,
+					false);
+		}
+		ret = IIO_VAL_INT;
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		*val = temperature_state->scale_pre_decml;
+		*val2 = temperature_state->scale_post_decml;
+		ret = temperature_state->scale_precision;
+		break;
+	case IIO_CHAN_INFO_OFFSET:
+		*val = temperature_state->value_offset;
+		ret = IIO_VAL_INT;
+		break;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = hid_sensor_read_samp_freq_value(
+				&temperature_state->common_attributes,
+				val, val2);
+		break;
+	case IIO_CHAN_INFO_HYSTERESIS:
+		ret = hid_sensor_read_raw_hyst_value(
+				&temperature_state->common_attributes,
+				val, val2);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+/* Channel write_raw handler */
+static int temperature_write_raw(struct iio_dev *indio_dev,
+				struct iio_chan_spec const *chan,
+				int val, int val2, long mask)
+{
+	struct temperature_state *temperature_state = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = hid_sensor_write_samp_freq_value(
+				&temperature_state->common_attributes, val,
+				val2);
+		break;
+	case IIO_CHAN_INFO_HYSTERESIS:
+		ret = hid_sensor_write_raw_hyst_value(
+				&temperature_state->common_attributes, val,
+				val2);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static const struct iio_info temperature_info = {
+	.driver_module = THIS_MODULE,
+	.read_raw = &temperature_read_raw,
+	.write_raw = &temperature_write_raw,
+};
+
+/* Callback handler to send event after all samples are received and captured */
+static int temperature_proc_event(struct hid_sensor_hub_device *hsdev,
+				unsigned int usage_id, void *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct temperature_state *temperature_state = iio_priv(indio_dev);
+
+	if (atomic_read(&temperature_state->common_attributes.data_ready))
+		iio_push_to_buffers(indio_dev,
+					&temperature_state->temperature_data);
+	return 0;
+}
+
+/* Capture samples in local storage */
+static int temperature_capture_sample(struct hid_sensor_hub_device *hsdev,
+				unsigned int usage_id, size_t raw_len,
+				char *raw_data, void *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct temperature_state *temperature_state = iio_priv(indio_dev);
+
+	switch (usage_id) {
+	case HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE:
+		temperature_state->temperature_data = *(s32 *)raw_data;
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+/* Parse report which is specific to an usage id*/
+static int temperature_parse_report(struct platform_device *pdev,
+				struct hid_sensor_hub_device *hsdev,
+				struct iio_chan_spec *channels,
+				unsigned int usage_id,
+				struct temperature_state *st)
+{
+	int ret;
+
+	ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT,
+			usage_id,
+			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
+			&st->temperature_attr);
+	if (ret < 0)
+		return ret;
+
+	temperature_adjust_channel_bit_mask(channels, 0,
+					st->temperature_attr.size);
+
+	st->scale_precision = hid_sensor_format_scale(
+				HID_USAGE_SENSOR_TEMPERATURE,
+				&st->temperature_attr,
+				&st->scale_pre_decml, &st->scale_post_decml);
+
+	/* Set Sensitivity field ids, when there is no individual modifier */
+	if (st->common_attributes.sensitivity.index < 0)
+		sensor_hub_input_get_attribute_info(hsdev,
+			HID_FEATURE_REPORT, usage_id,
+			HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS |
+			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
+			&st->common_attributes.sensitivity);
+
+	return ret;
+}
+
+static struct hid_sensor_hub_callbacks temperature_callbacks = {
+	.send_event = &temperature_proc_event,
+	.capture_sample = &temperature_capture_sample,
+};
+
+/* Function to initialize the processing for usage id */
+static int hid_temperature_probe(struct platform_device *pdev)
+{
+	static const char *name = "temperature";
+	struct iio_dev *indio_dev;
+	struct temperature_state *temperature_state;
+	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(&pdev->dev,
+				sizeof(struct temperature_state));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	temperature_state = iio_priv(indio_dev);
+	temperature_state->common_attributes.hsdev = hsdev;
+	temperature_state->common_attributes.pdev = pdev;
+
+	ret = hid_sensor_parse_common_attributes(hsdev,
+					HID_USAGE_SENSOR_TEMPERATURE,
+					&temperature_state->common_attributes);
+	if (ret)
+		return ret;
+
+	indio_dev->channels = devm_kmemdup(&indio_dev->dev,
+					temperature_channels,
+					sizeof(temperature_channels),
+					GFP_KERNEL);
+	if (!indio_dev->channels)
+		return -ENOMEM;
+
+	ret = temperature_parse_report(pdev, hsdev,
+				(struct iio_chan_spec *)indio_dev->channels,
+				HID_USAGE_SENSOR_TEMPERATURE,
+				temperature_state);
+	if (ret)
+		return ret;
+
+	indio_dev->num_channels = ARRAY_SIZE(temperature_channels);
+	indio_dev->dev.parent = &pdev->dev;
+	indio_dev->info = &temperature_info;
+	indio_dev->name = name;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	ret = devm_iio_triggered_buffer_setup(&pdev->dev, indio_dev,
+					&iio_pollfunc_store_time,
+					NULL, NULL);
+	if (ret)
+		return ret;
+
+	atomic_set(&temperature_state->common_attributes.data_ready, 0);
+	ret = hid_sensor_setup_trigger(indio_dev, name,
+				&temperature_state->common_attributes);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	temperature_callbacks.pdev = pdev;
+	ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE,
+					&temperature_callbacks);
+	if (ret)
+		goto error_remove_trigger;
+
+	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
+	if (ret)
+		goto error_remove_callback;
+
+	return ret;
+
+error_remove_callback:
+	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
+error_remove_trigger:
+	hid_sensor_remove_trigger(&temperature_state->common_attributes);
+	return ret;
+}
+
+/* Function to deinitialize the processing for usage id */
+static int hid_temperature_remove(struct platform_device *pdev)
+{
+	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct temperature_state *temperature_state = iio_priv(indio_dev);
+
+	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
+	hid_sensor_remove_trigger(&temperature_state->common_attributes);
+
+	return 0;
+}
+
+static const struct platform_device_id hid_temperature_ids[] = {
+	{
+		/* Format: HID-SENSOR-usage_id_in_hex_lowercase */
+		.name = "HID-SENSOR-200033",
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, hid_temperature_ids);
+
+static struct platform_driver hid_temperature_platform_driver = {
+	.id_table = hid_temperature_ids,
+	.driver = {
+		.name	= KBUILD_MODNAME,
+		.pm	= &hid_sensor_pm_ops,
+	},
+	.probe		= hid_temperature_probe,
+	.remove		= hid_temperature_remove,
+};
+module_platform_driver(hid_temperature_platform_driver);
+
+MODULE_DESCRIPTION("HID Environmental temperature sensor");
+MODULE_AUTHOR("Song Hongyan <hongyan.song@intel.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
index bab8375..ecec978 100644
--- a/include/linux/hid-sensor-ids.h
+++ b/include/linux/hid-sensor-ids.h
@@ -45,6 +45,10 @@
 #define HID_USAGE_SENSOR_DATA_ATMOSPHERIC_PRESSURE              0x200430
 #define HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE                   0x200431
 
+/* Tempreture (200033) */
+#define	HID_USAGE_SENSOR_TEMPERATURE				0x200033
+#define	HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE		0x200434
+
 /* Gyro 3D: (200076) */
 #define HID_USAGE_SENSOR_GYRO_3D				0x200076
 #define HID_USAGE_SENSOR_DATA_ANGL_VELOCITY			0x200456
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] iio: hid: Add temperature sensor support
  2017-02-13  9:49 [PATCH v3] iio: hid: Add temperature sensor support Song Hongyan
@ 2017-02-19 11:48     ` Jonathan Cameron
  0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2017-02-19 11:48 UTC (permalink / raw)
  To: Song Hongyan, linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA
  Cc: jikos-DgEjT+Ai2ygdnm+yROfE0A, srinivas.pandruvada-ral2JQCrhuEAvxtiuMwx3w

On 13/02/17 09:49, Song Hongyan wrote:
> Environmental temperature sensor is a hid defined sensor,
> it measures temperature.
> 
> More information can be found in:
> http://www.usb.org/developers/hidpage/HUTRR39b.pdf
> 
> According to IIO ABI definition, IIO_TEMP data output unit is
> milli degrees Celsius. Add the unit convert from degree to milli degree.
> 
> Signed-off-by: Song Hongyan <hongyan.song-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Hi Song,

I'm afraid I haven't really been taking a close look at the previous
versions so a few comments inline from me that might otherwise have
come on earlier versions.

I will be looking for a reviewed-by or Ack from Srinivas on this
before taking it (mostly to reflect the effort he has put in on those
earlier reviews!)

Jonathan
> ---
>  .../iio/common/hid-sensors/hid-sensor-attributes.c |   3 +
>  drivers/iio/temperature/Kconfig                    |  14 +
>  drivers/iio/temperature/Makefile                   |   1 +
>  drivers/iio/temperature/hid-sensor-temperature.c   | 326 +++++++++++++++++++++
>  include/linux/hid-sensor-ids.h                     |   4 +
>  5 files changed, 348 insertions(+)
>  create mode 100644 drivers/iio/temperature/hid-sensor-temperature.c
> 
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> index 7ef94a9..68c2bb0 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> @@ -58,6 +58,9 @@
>  
>  	{HID_USAGE_SENSOR_PRESSURE, 0, 100, 0},
>  	{HID_USAGE_SENSOR_PRESSURE, HID_USAGE_SENSOR_UNITS_PASCAL, 0, 1000000},
> +
> +	{HID_USAGE_SENSOR_TEMPERATURE, 0, 1000, 0},
> +	{HID_USAGE_SENSOR_TEMPERATURE, HID_USAGE_SENSOR_UNITS_DEGREES, 1000, 0},
>  };
>  
>  static int pow_10(unsigned power)
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index 5ea77a7..5f7b9f7 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -19,6 +19,20 @@ config MAXIM_THERMOCOUPLE
>  	  This driver can also be built as a module. If so, the module will
>  	  be called maxim_thermocouple.
>  
> +config HID_SENSOR_TEMP
> +	tristate "HID Environmental temperature sensor"
> +	depends on HID_SENSOR_HUB
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	select HID_SENSOR_IIO_COMMON
> +	select HID_SENSOR_IIO_TRIGGER
> +	help
> +	  Say yes here to build support for the HID SENSOR
> +	  temperature driver
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called hid-sensor-temperature.
> +
>  config MLX90614
>  	tristate "MLX90614 contact-less infrared sensor"
>  	depends on I2C
> diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> index 78c3de0..e157eda 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_MLX90614) += mlx90614.o
>  obj-$(CONFIG_TMP006) += tmp006.o
>  obj-$(CONFIG_TSYS01) += tsys01.o
>  obj-$(CONFIG_TSYS02D) += tsys02d.o
> +obj-$(CONFIG_HID_SENSOR_TEMP)   += hid-sensor-temperature.o
> diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
> new file mode 100644
> index 0000000..84abd2d
> --- /dev/null
> +++ b/drivers/iio/temperature/hid-sensor-temperature.c
> @@ -0,0 +1,326 @@
> +/* HID Sensors Driver
> + * Copyright (c) 2017, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.
> + *
> + */
> +#include <linux/device.h>
> +#include <linux/hid-sensor-hub.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include "../common/hid-sensors/hid-sensor-trigger.h"
> +
> +struct temperature_state {
> +	struct hid_sensor_common common_attributes;
> +	struct hid_sensor_hub_attribute_info temperature_attr;
> +	s32 temperature_data;
> +	int scale_pre_decml;
> +	int scale_post_decml;
> +	int scale_precision;
> +	int value_offset;
> +};
> +
> +/* Channel definitions */
> +static const struct iio_chan_spec temperature_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.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),
> +		.scan_index = 0,
> +	}
> +};
> +
> +/* Adjust channel real bits based on report descriptor */
> +static void temperature_adjust_channel_bit_mask(struct iio_chan_spec *channels,
> +					int channel, int size)
> +{
> +	channels[channel].scan_type.sign = 's';
> +	/* Real storage bits will change based on the report desc. */
> +	channels[channel].scan_type.realbits = size * 8;
> +	/* Maximum size of a sample to capture is s32 */
> +	channels[channel].scan_type.storagebits = sizeof(s32) * 8;
> +}
> +
> +/* Channel read_raw handler */
> +static int temperature_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val, int *val2, long mask)
> +{
> +	struct temperature_state *temperature_state = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (chan->type == IIO_TEMP) {
> +			hid_sensor_power_state(
> +				&temperature_state->common_attributes, true);
> +			*val = sensor_hub_input_attr_get_raw_value(
> +				temperature_state->common_attributes.hsdev,
> +				HID_USAGE_SENSOR_TEMPERATURE,
> +				HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
> +				temperature_state->temperature_attr.report_id,
> +				SENSOR_HUB_SYNC);
> +			hid_sensor_power_state(
> +					&temperature_state->common_attributes,
> +					false);
> +		}
This is an oddity.  If you get here without chan->type == IIO_TEMP
(which you can't, but your code is kind of assuming you can)
then you are returning that the data is valid without setting it to
anything.

I'd flip the logic
if (chan->type != IIO_TEMP) {
   return -EINVAL;
}
... normal code flow ...
> +		ret = IIO_VAL_INT;
There is nothing other than a return to be done after this so just
do it directly here instead.  Basic rule of thumb in kernel code is
return as early as possible when there is no unwinding to be done.
Same in all the branches of this switch statement pelase.
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = temperature_state->scale_pre_decml;
> +		*val2 = temperature_state->scale_post_decml;
> +		ret = temperature_state->scale_precision;
> +		break;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = temperature_state->value_offset;
> +		ret = IIO_VAL_INT;
> +		break;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = hid_sensor_read_samp_freq_value(
> +				&temperature_state->common_attributes,
> +				val, val2);
> +		break;
> +	case IIO_CHAN_INFO_HYSTERESIS:
> +		ret = hid_sensor_read_raw_hyst_value(
> +				&temperature_state->common_attributes,
> +				val, val2);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +/* Channel write_raw handler */
Perhaps drop the more obvious comments as not necessarily providing
any additional info.  I don't really mind if you feel they are useful
however and want to keep them.
> +static int temperature_write_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int val, int val2, long mask)
> +{
> +	struct temperature_state *temperature_state = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = hid_sensor_write_samp_freq_value(
> +				&temperature_state->common_attributes, val,
> +				val2);
As with the read_raw above, return directly within these statements rather than
having to bother with breaking out of the switch and then doing it.
Tidier code that way.

return hid_sensor....
> +		break;
> +	case IIO_CHAN_INFO_HYSTERESIS:
> +		ret = hid_sensor_write_raw_hyst_value(
> +				&temperature_state->common_attributes, val,
> +				val2);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info temperature_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = &temperature_read_raw,
> +	.write_raw = &temperature_write_raw,
> +};
> +
> +/* Callback handler to send event after all samples are received and captured */
> +static int temperature_proc_event(struct hid_sensor_hub_device *hsdev,
> +				unsigned int usage_id, void *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct temperature_state *temperature_state = iio_priv(indio_dev);
> +
> +	if (atomic_read(&temperature_state->common_attributes.data_ready))
> +		iio_push_to_buffers(indio_dev,
> +					&temperature_state->temperature_data);
> +	return 0;
> +}
> +
> +/* Capture samples in local storage */
> +static int temperature_capture_sample(struct hid_sensor_hub_device *hsdev,
> +				unsigned int usage_id, size_t raw_len,
> +				char *raw_data, void *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct temperature_state *temperature_state = iio_priv(indio_dev);
> +
> +	switch (usage_id) {
> +	case HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE:
> +		temperature_state->temperature_data = *(s32 *)raw_data;
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +/* Parse report which is specific to an usage id*/
> +static int temperature_parse_report(struct platform_device *pdev,
> +				struct hid_sensor_hub_device *hsdev,
> +				struct iio_chan_spec *channels,
> +				unsigned int usage_id,
> +				struct temperature_state *st)
> +{
> +	int ret;
> +
> +	ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT,
> +			usage_id,
> +			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
> +			&st->temperature_attr);
> +	if (ret < 0)
> +		return ret;
> +
> +	temperature_adjust_channel_bit_mask(channels, 0,
> +					st->temperature_attr.size);
> +
> +	st->scale_precision = hid_sensor_format_scale(
> +				HID_USAGE_SENSOR_TEMPERATURE,
> +				&st->temperature_attr,
> +				&st->scale_pre_decml, &st->scale_post_decml);
> +
> +	/* Set Sensitivity field ids, when there is no individual modifier */
> +	if (st->common_attributes.sensitivity.index < 0)
> +		sensor_hub_input_get_attribute_info(hsdev,
> +			HID_FEATURE_REPORT, usage_id,
> +			HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS |
> +			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
> +			&st->common_attributes.sensitivity);
> +
> +	return ret;
> +}
> +
> +static struct hid_sensor_hub_callbacks temperature_callbacks = {
> +	.send_event = &temperature_proc_event,
> +	.capture_sample = &temperature_capture_sample,
> +};
> +
> +/* Function to initialize the processing for usage id */
> +static int hid_temperature_probe(struct platform_device *pdev)
> +{
> +	static const char *name = "temperature";
> +	struct iio_dev *indio_dev;
> +	struct temperature_state *temperature_state;
Maybe shorten the variable name to lead to some shorter lines later?
temp_st for example would I think convey enough meaning.

> +	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev,
> +				sizeof(struct temperature_state));
Ever so slight preference for sizeof(*temperature_state)

> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	temperature_state = iio_priv(indio_dev);
> +	temperature_state->common_attributes.hsdev = hsdev;
> +	temperature_state->common_attributes.pdev = pdev;
> +
> +	ret = hid_sensor_parse_common_attributes(hsdev,
> +					HID_USAGE_SENSOR_TEMPERATURE,
> +					&temperature_state->common_attributes);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->channels = devm_kmemdup(&indio_dev->dev,
maybe rename as temp_chans as verbosity not adding much here.
> +					temperature_channels,
> +					sizeof(temperature_channels),
> +					GFP_KERNEL);
> +	if (!indio_dev->channels)
> +		return -ENOMEM;
> +
> +	ret = temperature_parse_report(pdev, hsdev,
> +				(struct iio_chan_spec *)indio_dev->channels,

This is casting away a const, which I don't particularly like.
I'd prefer that you used a local variable, got everthing setup as you like
and then assigned indio_dev->channels to it.  Semantically slightly nicer as
assumption is that they are const if accessed via this element of struct
iio_dev (here we obviously know we can change them but it's still not nice!)

> +				HID_USAGE_SENSOR_TEMPERATURE,
> +				temperature_state);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->num_channels = ARRAY_SIZE(temperature_channels);
> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->info = &temperature_info;
> +	indio_dev->name = name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = devm_iio_triggered_buffer_setup(&pdev->dev, indio_dev,
> +					&iio_pollfunc_store_time,
Oddity here as you don't actually use the timestamp...
> +					NULL, NULL);
> +	if (ret)
> +		return ret;
> +
> +	atomic_set(&temperature_state->common_attributes.data_ready, 0);
> +	ret = hid_sensor_setup_trigger(indio_dev, name,
> +				&temperature_state->common_attributes);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	temperature_callbacks.pdev = pdev;
> +	ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE,
> +					&temperature_callbacks);
> +	if (ret)
> +		goto error_remove_trigger;
> +
> +	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> +	if (ret)
> +		goto error_remove_callback;
> +
> +	return ret;
> +
> +error_remove_callback:
> +	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
> +error_remove_trigger:
> +	hid_sensor_remove_trigger(&temperature_state->common_attributes);
> +	return ret;
> +}
> +
> +/* Function to deinitialize the processing for usage id */
> +static int hid_temperature_remove(struct platform_device *pdev)
> +{
> +	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
Is it just me that feels that for consistence there should be a
platform_get_platform_data() function?
A well, can't say it would really add anything, just my sense of
consistency kicking in ;)
Anyhow, not really anything to do with this driver review so feel
free to ignore!

> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct temperature_state *temperature_state = iio_priv(indio_dev);
> +
> +	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
> +	hid_sensor_remove_trigger(&temperature_state->common_attributes);
> +
> +	return 0;
> +}
> +
> +static const struct platform_device_id hid_temperature_ids[] = {
> +	{
> +		/* Format: HID-SENSOR-usage_id_in_hex_lowercase */
> +		.name = "HID-SENSOR-200033",
> +	},
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, hid_temperature_ids);
> +
> +static struct platform_driver hid_temperature_platform_driver = {
> +	.id_table = hid_temperature_ids,
> +	.driver = {
> +		.name	= KBUILD_MODNAME,
> +		.pm	= &hid_sensor_pm_ops,
> +	},
> +	.probe		= hid_temperature_probe,
> +	.remove		= hid_temperature_remove,
> +};
> +module_platform_driver(hid_temperature_platform_driver);
> +
> +MODULE_DESCRIPTION("HID Environmental temperature sensor");
> +MODULE_AUTHOR("Song Hongyan <hongyan.song-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
> index bab8375..ecec978 100644
> --- a/include/linux/hid-sensor-ids.h
> +++ b/include/linux/hid-sensor-ids.h
> @@ -45,6 +45,10 @@
>  #define HID_USAGE_SENSOR_DATA_ATMOSPHERIC_PRESSURE              0x200430
>  #define HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE                   0x200431
>  
> +/* Tempreture (200033) */
> +#define	HID_USAGE_SENSOR_TEMPERATURE				0x200033
> +#define	HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE		0x200434
> +
>  /* Gyro 3D: (200076) */
>  #define HID_USAGE_SENSOR_GYRO_3D				0x200076
>  #define HID_USAGE_SENSOR_DATA_ANGL_VELOCITY			0x200456
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] iio: hid: Add temperature sensor support
@ 2017-02-19 11:48     ` Jonathan Cameron
  0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2017-02-19 11:48 UTC (permalink / raw)
  To: Song Hongyan, linux-input, linux-iio; +Cc: jikos, srinivas.pandruvada

On 13/02/17 09:49, Song Hongyan wrote:
> Environmental temperature sensor is a hid defined sensor,
> it measures temperature.
> 
> More information can be found in:
> http://www.usb.org/developers/hidpage/HUTRR39b.pdf
> 
> According to IIO ABI definition, IIO_TEMP data output unit is
> milli degrees Celsius. Add the unit convert from degree to milli degree.
> 
> Signed-off-by: Song Hongyan <hongyan.song@intel.com>
Hi Song,

I'm afraid I haven't really been taking a close look at the previous
versions so a few comments inline from me that might otherwise have
come on earlier versions.

I will be looking for a reviewed-by or Ack from Srinivas on this
before taking it (mostly to reflect the effort he has put in on those
earlier reviews!)

Jonathan
> ---
>  .../iio/common/hid-sensors/hid-sensor-attributes.c |   3 +
>  drivers/iio/temperature/Kconfig                    |  14 +
>  drivers/iio/temperature/Makefile                   |   1 +
>  drivers/iio/temperature/hid-sensor-temperature.c   | 326 +++++++++++++++++++++
>  include/linux/hid-sensor-ids.h                     |   4 +
>  5 files changed, 348 insertions(+)
>  create mode 100644 drivers/iio/temperature/hid-sensor-temperature.c
> 
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> index 7ef94a9..68c2bb0 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> @@ -58,6 +58,9 @@
>  
>  	{HID_USAGE_SENSOR_PRESSURE, 0, 100, 0},
>  	{HID_USAGE_SENSOR_PRESSURE, HID_USAGE_SENSOR_UNITS_PASCAL, 0, 1000000},
> +
> +	{HID_USAGE_SENSOR_TEMPERATURE, 0, 1000, 0},
> +	{HID_USAGE_SENSOR_TEMPERATURE, HID_USAGE_SENSOR_UNITS_DEGREES, 1000, 0},
>  };
>  
>  static int pow_10(unsigned power)
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index 5ea77a7..5f7b9f7 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -19,6 +19,20 @@ config MAXIM_THERMOCOUPLE
>  	  This driver can also be built as a module. If so, the module will
>  	  be called maxim_thermocouple.
>  
> +config HID_SENSOR_TEMP
> +	tristate "HID Environmental temperature sensor"
> +	depends on HID_SENSOR_HUB
> +	select IIO_BUFFER
> +	select IIO_TRIGGERED_BUFFER
> +	select HID_SENSOR_IIO_COMMON
> +	select HID_SENSOR_IIO_TRIGGER
> +	help
> +	  Say yes here to build support for the HID SENSOR
> +	  temperature driver
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called hid-sensor-temperature.
> +
>  config MLX90614
>  	tristate "MLX90614 contact-less infrared sensor"
>  	depends on I2C
> diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> index 78c3de0..e157eda 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_MLX90614) += mlx90614.o
>  obj-$(CONFIG_TMP006) += tmp006.o
>  obj-$(CONFIG_TSYS01) += tsys01.o
>  obj-$(CONFIG_TSYS02D) += tsys02d.o
> +obj-$(CONFIG_HID_SENSOR_TEMP)   += hid-sensor-temperature.o
> diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
> new file mode 100644
> index 0000000..84abd2d
> --- /dev/null
> +++ b/drivers/iio/temperature/hid-sensor-temperature.c
> @@ -0,0 +1,326 @@
> +/* HID Sensors Driver
> + * Copyright (c) 2017, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.
> + *
> + */
> +#include <linux/device.h>
> +#include <linux/hid-sensor-hub.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include "../common/hid-sensors/hid-sensor-trigger.h"
> +
> +struct temperature_state {
> +	struct hid_sensor_common common_attributes;
> +	struct hid_sensor_hub_attribute_info temperature_attr;
> +	s32 temperature_data;
> +	int scale_pre_decml;
> +	int scale_post_decml;
> +	int scale_precision;
> +	int value_offset;
> +};
> +
> +/* Channel definitions */
> +static const struct iio_chan_spec temperature_channels[] = {
> +	{
> +		.type = IIO_TEMP,
> +		.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),
> +		.scan_index = 0,
> +	}
> +};
> +
> +/* Adjust channel real bits based on report descriptor */
> +static void temperature_adjust_channel_bit_mask(struct iio_chan_spec *channels,
> +					int channel, int size)
> +{
> +	channels[channel].scan_type.sign = 's';
> +	/* Real storage bits will change based on the report desc. */
> +	channels[channel].scan_type.realbits = size * 8;
> +	/* Maximum size of a sample to capture is s32 */
> +	channels[channel].scan_type.storagebits = sizeof(s32) * 8;
> +}
> +
> +/* Channel read_raw handler */
> +static int temperature_read_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int *val, int *val2, long mask)
> +{
> +	struct temperature_state *temperature_state = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (chan->type == IIO_TEMP) {
> +			hid_sensor_power_state(
> +				&temperature_state->common_attributes, true);
> +			*val = sensor_hub_input_attr_get_raw_value(
> +				temperature_state->common_attributes.hsdev,
> +				HID_USAGE_SENSOR_TEMPERATURE,
> +				HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
> +				temperature_state->temperature_attr.report_id,
> +				SENSOR_HUB_SYNC);
> +			hid_sensor_power_state(
> +					&temperature_state->common_attributes,
> +					false);
> +		}
This is an oddity.  If you get here without chan->type == IIO_TEMP
(which you can't, but your code is kind of assuming you can)
then you are returning that the data is valid without setting it to
anything.

I'd flip the logic
if (chan->type != IIO_TEMP) {
   return -EINVAL;
}
... normal code flow ...
> +		ret = IIO_VAL_INT;
There is nothing other than a return to be done after this so just
do it directly here instead.  Basic rule of thumb in kernel code is
return as early as possible when there is no unwinding to be done.
Same in all the branches of this switch statement pelase.
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = temperature_state->scale_pre_decml;
> +		*val2 = temperature_state->scale_post_decml;
> +		ret = temperature_state->scale_precision;
> +		break;
> +	case IIO_CHAN_INFO_OFFSET:
> +		*val = temperature_state->value_offset;
> +		ret = IIO_VAL_INT;
> +		break;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = hid_sensor_read_samp_freq_value(
> +				&temperature_state->common_attributes,
> +				val, val2);
> +		break;
> +	case IIO_CHAN_INFO_HYSTERESIS:
> +		ret = hid_sensor_read_raw_hyst_value(
> +				&temperature_state->common_attributes,
> +				val, val2);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +/* Channel write_raw handler */
Perhaps drop the more obvious comments as not necessarily providing
any additional info.  I don't really mind if you feel they are useful
however and want to keep them.
> +static int temperature_write_raw(struct iio_dev *indio_dev,
> +				struct iio_chan_spec const *chan,
> +				int val, int val2, long mask)
> +{
> +	struct temperature_state *temperature_state = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = hid_sensor_write_samp_freq_value(
> +				&temperature_state->common_attributes, val,
> +				val2);
As with the read_raw above, return directly within these statements rather than
having to bother with breaking out of the switch and then doing it.
Tidier code that way.

return hid_sensor....
> +		break;
> +	case IIO_CHAN_INFO_HYSTERESIS:
> +		ret = hid_sensor_write_raw_hyst_value(
> +				&temperature_state->common_attributes, val,
> +				val2);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	return ret;
> +}
> +
> +static const struct iio_info temperature_info = {
> +	.driver_module = THIS_MODULE,
> +	.read_raw = &temperature_read_raw,
> +	.write_raw = &temperature_write_raw,
> +};
> +
> +/* Callback handler to send event after all samples are received and captured */
> +static int temperature_proc_event(struct hid_sensor_hub_device *hsdev,
> +				unsigned int usage_id, void *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct temperature_state *temperature_state = iio_priv(indio_dev);
> +
> +	if (atomic_read(&temperature_state->common_attributes.data_ready))
> +		iio_push_to_buffers(indio_dev,
> +					&temperature_state->temperature_data);
> +	return 0;
> +}
> +
> +/* Capture samples in local storage */
> +static int temperature_capture_sample(struct hid_sensor_hub_device *hsdev,
> +				unsigned int usage_id, size_t raw_len,
> +				char *raw_data, void *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct temperature_state *temperature_state = iio_priv(indio_dev);
> +
> +	switch (usage_id) {
> +	case HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE:
> +		temperature_state->temperature_data = *(s32 *)raw_data;
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +/* Parse report which is specific to an usage id*/
> +static int temperature_parse_report(struct platform_device *pdev,
> +				struct hid_sensor_hub_device *hsdev,
> +				struct iio_chan_spec *channels,
> +				unsigned int usage_id,
> +				struct temperature_state *st)
> +{
> +	int ret;
> +
> +	ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT,
> +			usage_id,
> +			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
> +			&st->temperature_attr);
> +	if (ret < 0)
> +		return ret;
> +
> +	temperature_adjust_channel_bit_mask(channels, 0,
> +					st->temperature_attr.size);
> +
> +	st->scale_precision = hid_sensor_format_scale(
> +				HID_USAGE_SENSOR_TEMPERATURE,
> +				&st->temperature_attr,
> +				&st->scale_pre_decml, &st->scale_post_decml);
> +
> +	/* Set Sensitivity field ids, when there is no individual modifier */
> +	if (st->common_attributes.sensitivity.index < 0)
> +		sensor_hub_input_get_attribute_info(hsdev,
> +			HID_FEATURE_REPORT, usage_id,
> +			HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS |
> +			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
> +			&st->common_attributes.sensitivity);
> +
> +	return ret;
> +}
> +
> +static struct hid_sensor_hub_callbacks temperature_callbacks = {
> +	.send_event = &temperature_proc_event,
> +	.capture_sample = &temperature_capture_sample,
> +};
> +
> +/* Function to initialize the processing for usage id */
> +static int hid_temperature_probe(struct platform_device *pdev)
> +{
> +	static const char *name = "temperature";
> +	struct iio_dev *indio_dev;
> +	struct temperature_state *temperature_state;
Maybe shorten the variable name to lead to some shorter lines later?
temp_st for example would I think convey enough meaning.

> +	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev,
> +				sizeof(struct temperature_state));
Ever so slight preference for sizeof(*temperature_state)

> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	temperature_state = iio_priv(indio_dev);
> +	temperature_state->common_attributes.hsdev = hsdev;
> +	temperature_state->common_attributes.pdev = pdev;
> +
> +	ret = hid_sensor_parse_common_attributes(hsdev,
> +					HID_USAGE_SENSOR_TEMPERATURE,
> +					&temperature_state->common_attributes);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->channels = devm_kmemdup(&indio_dev->dev,
maybe rename as temp_chans as verbosity not adding much here.
> +					temperature_channels,
> +					sizeof(temperature_channels),
> +					GFP_KERNEL);
> +	if (!indio_dev->channels)
> +		return -ENOMEM;
> +
> +	ret = temperature_parse_report(pdev, hsdev,
> +				(struct iio_chan_spec *)indio_dev->channels,

This is casting away a const, which I don't particularly like.
I'd prefer that you used a local variable, got everthing setup as you like
and then assigned indio_dev->channels to it.  Semantically slightly nicer as
assumption is that they are const if accessed via this element of struct
iio_dev (here we obviously know we can change them but it's still not nice!)

> +				HID_USAGE_SENSOR_TEMPERATURE,
> +				temperature_state);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->num_channels = ARRAY_SIZE(temperature_channels);
> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->info = &temperature_info;
> +	indio_dev->name = name;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +
> +	ret = devm_iio_triggered_buffer_setup(&pdev->dev, indio_dev,
> +					&iio_pollfunc_store_time,
Oddity here as you don't actually use the timestamp...
> +					NULL, NULL);
> +	if (ret)
> +		return ret;
> +
> +	atomic_set(&temperature_state->common_attributes.data_ready, 0);
> +	ret = hid_sensor_setup_trigger(indio_dev, name,
> +				&temperature_state->common_attributes);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	temperature_callbacks.pdev = pdev;
> +	ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE,
> +					&temperature_callbacks);
> +	if (ret)
> +		goto error_remove_trigger;
> +
> +	ret = devm_iio_device_register(indio_dev->dev.parent, indio_dev);
> +	if (ret)
> +		goto error_remove_callback;
> +
> +	return ret;
> +
> +error_remove_callback:
> +	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
> +error_remove_trigger:
> +	hid_sensor_remove_trigger(&temperature_state->common_attributes);
> +	return ret;
> +}
> +
> +/* Function to deinitialize the processing for usage id */
> +static int hid_temperature_remove(struct platform_device *pdev)
> +{
> +	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
Is it just me that feels that for consistence there should be a
platform_get_platform_data() function?
A well, can't say it would really add anything, just my sense of
consistency kicking in ;)
Anyhow, not really anything to do with this driver review so feel
free to ignore!

> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct temperature_state *temperature_state = iio_priv(indio_dev);
> +
> +	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TEMPERATURE);
> +	hid_sensor_remove_trigger(&temperature_state->common_attributes);
> +
> +	return 0;
> +}
> +
> +static const struct platform_device_id hid_temperature_ids[] = {
> +	{
> +		/* Format: HID-SENSOR-usage_id_in_hex_lowercase */
> +		.name = "HID-SENSOR-200033",
> +	},
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, hid_temperature_ids);
> +
> +static struct platform_driver hid_temperature_platform_driver = {
> +	.id_table = hid_temperature_ids,
> +	.driver = {
> +		.name	= KBUILD_MODNAME,
> +		.pm	= &hid_sensor_pm_ops,
> +	},
> +	.probe		= hid_temperature_probe,
> +	.remove		= hid_temperature_remove,
> +};
> +module_platform_driver(hid_temperature_platform_driver);
> +
> +MODULE_DESCRIPTION("HID Environmental temperature sensor");
> +MODULE_AUTHOR("Song Hongyan <hongyan.song@intel.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h
> index bab8375..ecec978 100644
> --- a/include/linux/hid-sensor-ids.h
> +++ b/include/linux/hid-sensor-ids.h
> @@ -45,6 +45,10 @@
>  #define HID_USAGE_SENSOR_DATA_ATMOSPHERIC_PRESSURE              0x200430
>  #define HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE                   0x200431
>  
> +/* Tempreture (200033) */
> +#define	HID_USAGE_SENSOR_TEMPERATURE				0x200033
> +#define	HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE		0x200434
> +
>  /* Gyro 3D: (200076) */
>  #define HID_USAGE_SENSOR_GYRO_3D				0x200076
>  #define HID_USAGE_SENSOR_DATA_ANGL_VELOCITY			0x200456
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] iio: hid: Add temperature sensor support
  2017-02-19 11:48     ` Jonathan Cameron
@ 2017-02-21 10:29       ` Pandruvada, Srinivas
  -1 siblings, 0 replies; 5+ messages in thread
From: Pandruvada, Srinivas @ 2017-02-21 10:29 UTC (permalink / raw)
  To: linux-input, Song, Hongyan, linux-iio, jic23; +Cc: jikos

On Sun, 2017-02-19 at 11:48 +0000, Jonathan Cameron wrote:
> On 13/02/17 09:49, Song Hongyan wrote:
> > Environmental temperature sensor is a hid defined sensor,
> > it measures temperature.
> > 
> > More information can be found in:
> > http://www.usb.org/developers/hidpage/HUTRR39b.pdf
> > 
> > According to IIO ABI definition, IIO_TEMP data output unit is
> > milli degrees Celsius. Add the unit convert from degree to milli
> > degree.
> > 
> > Signed-off-by: Song Hongyan <hongyan.song@intel.com>
> 
> Hi Song,
> 
> I'm afraid I haven't really been taking a close look at the previous
> versions so a few comments inline from me that might otherwise have
> come on earlier versions.
> 
> I will be looking for a reviewed-by or Ack from Srinivas on this
> before taking it (mostly to reflect the effort he has put in on those
> earlier reviews!)
Hongyan,

You can add

Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>

after addressing Jonathan's comments.

Thanks,
Srinivas

> 
> Jonathan
> > ---
> >  .../iio/common/hid-sensors/hid-sensor-attributes.c |   3 +
> >  drivers/iio/temperature/Kconfig                    |  14 +
> >  drivers/iio/temperature/Makefile                   |   1 +
> >  drivers/iio/temperature/hid-sensor-temperature.c   | 326
> > +++++++++++++++++++++
> >  include/linux/hid-sensor-ids.h                     |   4 +
> >  5 files changed, 348 insertions(+)
> >  create mode 100644 drivers/iio/temperature/hid-sensor-
> > temperature.c
> > 
> > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c 
> > b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> > index 7ef94a9..68c2bb0 100644
> > --- a/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> > +++ b/drivers/iio/common/hid-sensors/hid-sensor-attributes.c
> > @@ -58,6 +58,9 @@
> >  
> >  	{HID_USAGE_SENSOR_PRESSURE, 0, 100, 0},
> >  	{HID_USAGE_SENSOR_PRESSURE, HID_USAGE_SENSOR_UNITS_PASCAL,
> > 0, 1000000},
> > +
> > +	{HID_USAGE_SENSOR_TEMPERATURE, 0, 1000, 0},
> > +	{HID_USAGE_SENSOR_TEMPERATURE,
> > HID_USAGE_SENSOR_UNITS_DEGREES, 1000, 0},
> >  };
> >  
> >  static int pow_10(unsigned power)
> > diff --git a/drivers/iio/temperature/Kconfig
> > b/drivers/iio/temperature/Kconfig
> > index 5ea77a7..5f7b9f7 100644
> > --- a/drivers/iio/temperature/Kconfig
> > +++ b/drivers/iio/temperature/Kconfig
> > @@ -19,6 +19,20 @@ config MAXIM_THERMOCOUPLE
> >  	  This driver can also be built as a module. If so, the
> > module will
> >  	  be called maxim_thermocouple.
> >  
> > +config HID_SENSOR_TEMP
> > +	tristate "HID Environmental temperature sensor"
> > +	depends on HID_SENSOR_HUB
> > +	select IIO_BUFFER
> > +	select IIO_TRIGGERED_BUFFER
> > +	select HID_SENSOR_IIO_COMMON
> > +	select HID_SENSOR_IIO_TRIGGER
> > +	help
> > +	  Say yes here to build support for the HID SENSOR
> > +	  temperature driver
> > +
> > +	  To compile this driver as a module, choose M here: the
> > module
> > +	  will be called hid-sensor-temperature.
> > +
> >  config MLX90614
> >  	tristate "MLX90614 contact-less infrared sensor"
> >  	depends on I2C
> > diff --git a/drivers/iio/temperature/Makefile
> > b/drivers/iio/temperature/Makefile
> > index 78c3de0..e157eda 100644
> > --- a/drivers/iio/temperature/Makefile
> > +++ b/drivers/iio/temperature/Makefile
> > @@ -7,3 +7,4 @@ obj-$(CONFIG_MLX90614) += mlx90614.o
> >  obj-$(CONFIG_TMP006) += tmp006.o
> >  obj-$(CONFIG_TSYS01) += tsys01.o
> >  obj-$(CONFIG_TSYS02D) += tsys02d.o
> > +obj-$(CONFIG_HID_SENSOR_TEMP)   += hid-sensor-temperature.o
> > diff --git a/drivers/iio/temperature/hid-sensor-temperature.c
> > b/drivers/iio/temperature/hid-sensor-temperature.c
> > new file mode 100644
> > index 0000000..84abd2d
> > --- /dev/null
> > +++ b/drivers/iio/temperature/hid-sensor-temperature.c
> > @@ -0,0 +1,326 @@
> > +/* HID Sensors Driver
> > + * Copyright (c) 2017, Intel Corporation.
> > + *
> > + * This program is free software; you can redistribute it and/or
> > modify it
> > + * under the terms and conditions of the GNU General Public
> > License,
> > + * version 2, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but
> > WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of
> > MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
> > License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public
> > License along with
> > + * this program.
> > + *
> > + */
> > +#include <linux/device.h>
> > +#include <linux/hid-sensor-hub.h>
> > +#include <linux/iio/buffer.h>
> > +#include <linux/iio/iio.h>
> > +#include <linux/iio/triggered_buffer.h>
> > +#include <linux/iio/trigger_consumer.h>
> > +#include <linux/module.h>
> > +#include <linux/platform_device.h>
> > +#include "../common/hid-sensors/hid-sensor-trigger.h"
> > +
> > +struct temperature_state {
> > +	struct hid_sensor_common common_attributes;
> > +	struct hid_sensor_hub_attribute_info temperature_attr;
> > +	s32 temperature_data;
> > +	int scale_pre_decml;
> > +	int scale_post_decml;
> > +	int scale_precision;
> > +	int value_offset;
> > +};
> > +
> > +/* Channel definitions */
> > +static const struct iio_chan_spec temperature_channels[] = {
> > +	{
> > +		.type = IIO_TEMP,
> > +		.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),
> > +		.scan_index = 0,
> > +	}
> > +};
> > +
> > +/* Adjust channel real bits based on report descriptor */
> > +static void temperature_adjust_channel_bit_mask(struct
> > iio_chan_spec *channels,
> > +					int channel, int size)
> > +{
> > +	channels[channel].scan_type.sign = 's';
> > +	/* Real storage bits will change based on the report desc.
> > */
> > +	channels[channel].scan_type.realbits = size * 8;
> > +	/* Maximum size of a sample to capture is s32 */
> > +	channels[channel].scan_type.storagebits = sizeof(s32) * 8;
> > +}
> > +
> > +/* Channel read_raw handler */
> > +static int temperature_read_raw(struct iio_dev *indio_dev,
> > +				struct iio_chan_spec const *chan,
> > +				int *val, int *val2, long mask)
> > +{
> > +	struct temperature_state *temperature_state =
> > iio_priv(indio_dev);
> > +	int ret;
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_RAW:
> > +		if (chan->type == IIO_TEMP) {
> > +			hid_sensor_power_state(
> > +				&temperature_state-
> > >common_attributes, true);
> > +			*val =
> > sensor_hub_input_attr_get_raw_value(
> > +				temperature_state-
> > >common_attributes.hsdev,
> > +				HID_USAGE_SENSOR_TEMPERATURE,
> > +				HID_USAGE_SENSOR_DATA_ENVIRONMENTA
> > L_TEMPERATURE,
> > +				temperature_state-
> > >temperature_attr.report_id,
> > +				SENSOR_HUB_SYNC);
> > +			hid_sensor_power_state(
> > +					&temperature_state-
> > >common_attributes,
> > +					false);
> > +		}
> 
> This is an oddity.  If you get here without chan->type == IIO_TEMP
> (which you can't, but your code is kind of assuming you can)
> then you are returning that the data is valid without setting it to
> anything.
> 
> I'd flip the logic
> if (chan->type != IIO_TEMP) {
>    return -EINVAL;
> }
> ... normal code flow ...
> > +		ret = IIO_VAL_INT;
> 
> There is nothing other than a return to be done after this so just
> do it directly here instead.  Basic rule of thumb in kernel code is
> return as early as possible when there is no unwinding to be done.
> Same in all the branches of this switch statement pelase.
> > +		break;
> > +	case IIO_CHAN_INFO_SCALE:
> > +		*val = temperature_state->scale_pre_decml;
> > +		*val2 = temperature_state->scale_post_decml;
> > +		ret = temperature_state->scale_precision;
> > +		break;
> > +	case IIO_CHAN_INFO_OFFSET:
> > +		*val = temperature_state->value_offset;
> > +		ret = IIO_VAL_INT;
> > +		break;
> > +	case IIO_CHAN_INFO_SAMP_FREQ:
> > +		ret = hid_sensor_read_samp_freq_value(
> > +				&temperature_state-
> > >common_attributes,
> > +				val, val2);
> > +		break;
> > +	case IIO_CHAN_INFO_HYSTERESIS:
> > +		ret = hid_sensor_read_raw_hyst_value(
> > +				&temperature_state-
> > >common_attributes,
> > +				val, val2);
> > +		break;
> > +	default:
> > +		ret = -EINVAL;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +/* Channel write_raw handler */
> 
> Perhaps drop the more obvious comments as not necessarily providing
> any additional info.  I don't really mind if you feel they are useful
> however and want to keep them.
> > +static int temperature_write_raw(struct iio_dev *indio_dev,
> > +				struct iio_chan_spec const *chan,
> > +				int val, int val2, long mask)
> > +{
> > +	struct temperature_state *temperature_state =
> > iio_priv(indio_dev);
> > +	int ret;
> > +
> > +	switch (mask) {
> > +	case IIO_CHAN_INFO_SAMP_FREQ:
> > +		ret = hid_sensor_write_samp_freq_value(
> > +				&temperature_state-
> > >common_attributes, val,
> > +				val2);
> 
> As with the read_raw above, return directly within these statements
> rather than
> having to bother with breaking out of the switch and then doing it.
> Tidier code that way.
> 
> return hid_sensor....
> > +		break;
> > +	case IIO_CHAN_INFO_HYSTERESIS:
> > +		ret = hid_sensor_write_raw_hyst_value(
> > +				&temperature_state-
> > >common_attributes, val,
> > +				val2);
> > +		break;
> > +	default:
> > +		ret = -EINVAL;
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> > +static const struct iio_info temperature_info = {
> > +	.driver_module = THIS_MODULE,
> > +	.read_raw = &temperature_read_raw,
> > +	.write_raw = &temperature_write_raw,
> > +};
> > +
> > +/* Callback handler to send event after all samples are received
> > and captured */
> > +static int temperature_proc_event(struct hid_sensor_hub_device
> > *hsdev,
> > +				unsigned int usage_id, void *pdev)
> > +{
> > +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > +	struct temperature_state *temperature_state =
> > iio_priv(indio_dev);
> > +
> > +	if (atomic_read(&temperature_state-
> > >common_attributes.data_ready))
> > +		iio_push_to_buffers(indio_dev,
> > +					&temperature_state-
> > >temperature_data);
> > +	return 0;
> > +}
> > +
> > +/* Capture samples in local storage */
> > +static int temperature_capture_sample(struct hid_sensor_hub_device
> > *hsdev,
> > +				unsigned int usage_id, size_t
> > raw_len,
> > +				char *raw_data, void *pdev)
> > +{
> > +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > +	struct temperature_state *temperature_state =
> > iio_priv(indio_dev);
> > +
> > +	switch (usage_id) {
> > +	case HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE:
> > +		temperature_state->temperature_data = *(s32
> > *)raw_data;
> > +		return 0;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +/* Parse report which is specific to an usage id*/
> > +static int temperature_parse_report(struct platform_device *pdev,
> > +				struct hid_sensor_hub_device
> > *hsdev,
> > +				struct iio_chan_spec *channels,
> > +				unsigned int usage_id,
> > +				struct temperature_state *st)
> > +{
> > +	int ret;
> > +
> > +	ret = sensor_hub_input_get_attribute_info(hsdev,
> > HID_INPUT_REPORT,
> > +			usage_id,
> > +			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPER
> > ATURE,
> > +			&st->temperature_attr);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	temperature_adjust_channel_bit_mask(channels, 0,
> > +					st-
> > >temperature_attr.size);
> > +
> > +	st->scale_precision = hid_sensor_format_scale(
> > +				HID_USAGE_SENSOR_TEMPERATURE,
> > +				&st->temperature_attr,
> > +				&st->scale_pre_decml, &st-
> > >scale_post_decml);
> > +
> > +	/* Set Sensitivity field ids, when there is no individual
> > modifier */
> > +	if (st->common_attributes.sensitivity.index < 0)
> > +		sensor_hub_input_get_attribute_info(hsdev,
> > +			HID_FEATURE_REPORT, usage_id,
> > +			HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVI
> > TY_ABS |
> > +			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPER
> > ATURE,
> > +			&st->common_attributes.sensitivity);
> > +
> > +	return ret;
> > +}
> > +
> > +static struct hid_sensor_hub_callbacks temperature_callbacks = {
> > +	.send_event = &temperature_proc_event,
> > +	.capture_sample = &temperature_capture_sample,
> > +};
> > +
> > +/* Function to initialize the processing for usage id */
> > +static int hid_temperature_probe(struct platform_device *pdev)
> > +{
> > +	static const char *name = "temperature";
> > +	struct iio_dev *indio_dev;
> > +	struct temperature_state *temperature_state;
> 
> Maybe shorten the variable name to lead to some shorter lines later?
> temp_st for example would I think convey enough meaning.
> 
> > +	struct hid_sensor_hub_device *hsdev = pdev-
> > >dev.platform_data;
> > +	int ret;
> > +
> > +	indio_dev = devm_iio_device_alloc(&pdev->dev,
> > +				sizeof(struct temperature_state));
> 
> Ever so slight preference for sizeof(*temperature_state)
> 
> > +	if (!indio_dev)
> > +		return -ENOMEM;
> > +
> > +	temperature_state = iio_priv(indio_dev);
> > +	temperature_state->common_attributes.hsdev = hsdev;
> > +	temperature_state->common_attributes.pdev = pdev;
> > +
> > +	ret = hid_sensor_parse_common_attributes(hsdev,
> > +					HID_USAGE_SENSOR_TEMPERATU
> > RE,
> > +					&temperature_state-
> > >common_attributes);
> > +	if (ret)
> > +		return ret;
> > +
> > +	indio_dev->channels = devm_kmemdup(&indio_dev->dev,
> 
> maybe rename as temp_chans as verbosity not adding much here.
> > +					temperature_channels,
> > +					sizeof(temperature_channel
> > s),
> > +					GFP_KERNEL);
> > +	if (!indio_dev->channels)
> > +		return -ENOMEM;
> > +
> > +	ret = temperature_parse_report(pdev, hsdev,
> > +				(struct iio_chan_spec *)indio_dev-
> > >channels,
> 
> This is casting away a const, which I don't particularly like.
> I'd prefer that you used a local variable, got everthing setup as you
> like
> and then assigned indio_dev->channels to it.  Semantically slightly
> nicer as
> assumption is that they are const if accessed via this element of
> struct
> iio_dev (here we obviously know we can change them but it's still not
> nice!)
> 
> > +				HID_USAGE_SENSOR_TEMPERATURE,
> > +				temperature_state);
> > +	if (ret)
> > +		return ret;
> > +
> > +	indio_dev->num_channels =
> > ARRAY_SIZE(temperature_channels);
> > +	indio_dev->dev.parent = &pdev->dev;
> > +	indio_dev->info = &temperature_info;
> > +	indio_dev->name = name;
> > +	indio_dev->modes = INDIO_DIRECT_MODE;
> > +
> > +	ret = devm_iio_triggered_buffer_setup(&pdev->dev,
> > indio_dev,
> > +					&iio_pollfunc_store_time,
> 
> Oddity here as you don't actually use the timestamp...
> > +					NULL, NULL);
> > +	if (ret)
> > +		return ret;
> > +
> > +	atomic_set(&temperature_state-
> > >common_attributes.data_ready, 0);
> > +	ret = hid_sensor_setup_trigger(indio_dev, name,
> > +				&temperature_state-
> > >common_attributes);
> > +	if (ret)
> > +		return ret;
> > +
> > +	platform_set_drvdata(pdev, indio_dev);
> > +
> > +	temperature_callbacks.pdev = pdev;
> > +	ret = sensor_hub_register_callback(hsdev,
> > HID_USAGE_SENSOR_TEMPERATURE,
> > +					&temperature_callbacks);
> > +	if (ret)
> > +		goto error_remove_trigger;
> > +
> > +	ret = devm_iio_device_register(indio_dev->dev.parent,
> > indio_dev);
> > +	if (ret)
> > +		goto error_remove_callback;
> > +
> > +	return ret;
> > +
> > +error_remove_callback:
> > +	sensor_hub_remove_callback(hsdev,
> > HID_USAGE_SENSOR_TEMPERATURE);
> > +error_remove_trigger:
> > +	hid_sensor_remove_trigger(&temperature_state-
> > >common_attributes);
> > +	return ret;
> > +}
> > +
> > +/* Function to deinitialize the processing for usage id */
> > +static int hid_temperature_remove(struct platform_device *pdev)
> > +{
> > +	struct hid_sensor_hub_device *hsdev = pdev-
> > >dev.platform_data;
> 
> Is it just me that feels that for consistence there should be a
> platform_get_platform_data() function?
> A well, can't say it would really add anything, just my sense of
> consistency kicking in ;)
> Anyhow, not really anything to do with this driver review so feel
> free to ignore!
> 
> > +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > +	struct temperature_state *temperature_state =
> > iio_priv(indio_dev);
> > +
> > +	sensor_hub_remove_callback(hsdev,
> > HID_USAGE_SENSOR_TEMPERATURE);
> > +	hid_sensor_remove_trigger(&temperature_state-
> > >common_attributes);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct platform_device_id hid_temperature_ids[] = {
> > +	{
> > +		/* Format: HID-SENSOR-usage_id_in_hex_lowercase */
> > +		.name = "HID-SENSOR-200033",
> > +	},
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(platform, hid_temperature_ids);
> > +
> > +static struct platform_driver hid_temperature_platform_driver = {
> > +	.id_table = hid_temperature_ids,
> > +	.driver = {
> > +		.name	= KBUILD_MODNAME,
> > +		.pm	= &hid_sensor_pm_ops,
> > +	},
> > +	.probe		= hid_temperature_probe,
> > +	.remove		= hid_temperature_remove,
> > +};
> > +module_platform_driver(hid_temperature_platform_driver);
> > +
> > +MODULE_DESCRIPTION("HID Environmental temperature sensor");
> > +MODULE_AUTHOR("Song Hongyan <hongyan.song@intel.com>");
> > +MODULE_LICENSE("GPL v2");
> > diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-
> > sensor-ids.h
> > index bab8375..ecec978 100644
> > --- a/include/linux/hid-sensor-ids.h
> > +++ b/include/linux/hid-sensor-ids.h
> > @@ -45,6 +45,10 @@
> >  #define
> > HID_USAGE_SENSOR_DATA_ATMOSPHERIC_PRESSURE              0x200430
> >  #define
> > HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE                   0x200431
> >  
> > +/* Tempreture (200033) */
> > +#define	HID_USAGE_SENSOR_TEMPERATURE			
> > 	0x200033
> > +#define	HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE	
> > 	0x200434
> > +
> >  /* Gyro 3D: (200076) */
> >  #define HID_USAGE_SENSOR_GYRO_3D				0x
> > 200076
> >  #define HID_USAGE_SENSOR_DATA_ANGL_VELOCITY			
> > 0x200456
> > 
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3] iio: hid: Add temperature sensor support
@ 2017-02-21 10:29       ` Pandruvada, Srinivas
  0 siblings, 0 replies; 5+ messages in thread
From: Pandruvada, Srinivas @ 2017-02-21 10:29 UTC (permalink / raw)
  To: linux-input, Song, Hongyan, linux-iio, jic23; +Cc: jikos

T24gU3VuLCAyMDE3LTAyLTE5IGF0IDExOjQ4ICswMDAwLCBKb25hdGhhbiBDYW1lcm9uIHdyb3Rl
Og0KPiBPbiAxMy8wMi8xNyAwOTo0OSwgU29uZyBIb25neWFuIHdyb3RlOg0KPiA+IEVudmlyb25t
ZW50YWwgdGVtcGVyYXR1cmUgc2Vuc29yIGlzIGEgaGlkIGRlZmluZWQgc2Vuc29yLA0KPiA+IGl0
IG1lYXN1cmVzIHRlbXBlcmF0dXJlLg0KPiA+IA0KPiA+IE1vcmUgaW5mb3JtYXRpb24gY2FuIGJl
IGZvdW5kIGluOg0KPiA+IGh0dHA6Ly93d3cudXNiLm9yZy9kZXZlbG9wZXJzL2hpZHBhZ2UvSFVU
UlIzOWIucGRmDQo+ID4gDQo+ID4gQWNjb3JkaW5nIHRvIElJTyBBQkkgZGVmaW5pdGlvbiwgSUlP
X1RFTVAgZGF0YSBvdXRwdXQgdW5pdCBpcw0KPiA+IG1pbGxpIGRlZ3JlZXMgQ2Vsc2l1cy4gQWRk
IHRoZSB1bml0IGNvbnZlcnQgZnJvbSBkZWdyZWUgdG8gbWlsbGkNCj4gPiBkZWdyZWUuDQo+ID4g
DQo+ID4gU2lnbmVkLW9mZi1ieTogU29uZyBIb25neWFuIDxob25neWFuLnNvbmdAaW50ZWwuY29t
Pg0KPiANCj4gSGkgU29uZywNCj4gDQo+IEknbSBhZnJhaWQgSSBoYXZlbid0IHJlYWxseSBiZWVu
IHRha2luZyBhIGNsb3NlIGxvb2sgYXQgdGhlIHByZXZpb3VzDQo+IHZlcnNpb25zIHNvIGEgZmV3
IGNvbW1lbnRzIGlubGluZSBmcm9tIG1lIHRoYXQgbWlnaHQgb3RoZXJ3aXNlIGhhdmUNCj4gY29t
ZSBvbiBlYXJsaWVyIHZlcnNpb25zLg0KPiANCj4gSSB3aWxsIGJlIGxvb2tpbmcgZm9yIGEgcmV2
aWV3ZWQtYnkgb3IgQWNrIGZyb20gU3Jpbml2YXMgb24gdGhpcw0KPiBiZWZvcmUgdGFraW5nIGl0
IChtb3N0bHkgdG8gcmVmbGVjdCB0aGUgZWZmb3J0IGhlIGhhcyBwdXQgaW4gb24gdGhvc2UNCj4g
ZWFybGllciByZXZpZXdzISkNCkhvbmd5YW4sDQoNCllvdSBjYW4gYWRkDQoNCkFja2VkLWJ5OiBT
cmluaXZhcyBQYW5kcnV2YWRhIDxzcmluaXZhcy5wYW5kcnV2YWRhQGxpbnV4LmludGVsLmNvbT4N
Cg0KYWZ0ZXIgYWRkcmVzc2luZyBKb25hdGhhbidzIGNvbW1lbnRzLg0KDQpUaGFua3MsDQpTcmlu
aXZhcw0KDQo+IA0KPiBKb25hdGhhbg0KPiA+IC0tLQ0KPiA+IMKgLi4uL2lpby9jb21tb24vaGlk
LXNlbnNvcnMvaGlkLXNlbnNvci1hdHRyaWJ1dGVzLmMgfMKgwqDCoDMgKw0KPiA+IMKgZHJpdmVy
cy9paW8vdGVtcGVyYXR1cmUvS2NvbmZpZ8KgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKg
wqDCoMKgwqB8wqDCoDE0ICsNCj4gPiDCoGRyaXZlcnMvaWlvL3RlbXBlcmF0dXJlL01ha2VmaWxl
wqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB8wqDCoMKgMSArDQo+ID4gwqBk
cml2ZXJzL2lpby90ZW1wZXJhdHVyZS9oaWQtc2Vuc29yLXRlbXBlcmF0dXJlLmPCoMKgwqB8IDMy
Ng0KPiA+ICsrKysrKysrKysrKysrKysrKysrKw0KPiA+IMKgaW5jbHVkZS9saW51eC9oaWQtc2Vu
c29yLWlkcy5owqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgfMKgwqDC
oDQgKw0KPiA+IMKgNSBmaWxlcyBjaGFuZ2VkLCAzNDggaW5zZXJ0aW9ucygrKQ0KPiA+IMKgY3Jl
YXRlIG1vZGUgMTAwNjQ0IGRyaXZlcnMvaWlvL3RlbXBlcmF0dXJlL2hpZC1zZW5zb3ItDQo+ID4g
dGVtcGVyYXR1cmUuYw0KPiA+IA0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2lpby9jb21tb24v
aGlkLXNlbnNvcnMvaGlkLXNlbnNvci1hdHRyaWJ1dGVzLmMgDQo+ID4gYi9kcml2ZXJzL2lpby9j
b21tb24vaGlkLXNlbnNvcnMvaGlkLXNlbnNvci1hdHRyaWJ1dGVzLmMNCj4gPiBpbmRleCA3ZWY5
NGE5Li42OGMyYmIwIDEwMDY0NA0KPiA+IC0tLSBhL2RyaXZlcnMvaWlvL2NvbW1vbi9oaWQtc2Vu
c29ycy9oaWQtc2Vuc29yLWF0dHJpYnV0ZXMuYw0KPiA+ICsrKyBiL2RyaXZlcnMvaWlvL2NvbW1v
bi9oaWQtc2Vuc29ycy9oaWQtc2Vuc29yLWF0dHJpYnV0ZXMuYw0KPiA+IEBAIC01OCw2ICs1OCw5
IEBADQo+ID4gwqANCj4gPiDCoAl7SElEX1VTQUdFX1NFTlNPUl9QUkVTU1VSRSwgMCwgMTAwLCAw
fSwNCj4gPiDCoAl7SElEX1VTQUdFX1NFTlNPUl9QUkVTU1VSRSwgSElEX1VTQUdFX1NFTlNPUl9V
TklUU19QQVNDQUwsDQo+ID4gMCwgMTAwMDAwMH0sDQo+ID4gKw0KPiA+ICsJe0hJRF9VU0FHRV9T
RU5TT1JfVEVNUEVSQVRVUkUsIDAsIDEwMDAsIDB9LA0KPiA+ICsJe0hJRF9VU0FHRV9TRU5TT1Jf
VEVNUEVSQVRVUkUsDQo+ID4gSElEX1VTQUdFX1NFTlNPUl9VTklUU19ERUdSRUVTLCAxMDAwLCAw
fSwNCj4gPiDCoH07DQo+ID4gwqANCj4gPiDCoHN0YXRpYyBpbnQgcG93XzEwKHVuc2lnbmVkIHBv
d2VyKQ0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2lpby90ZW1wZXJhdHVyZS9LY29uZmlnDQo+
ID4gYi9kcml2ZXJzL2lpby90ZW1wZXJhdHVyZS9LY29uZmlnDQo+ID4gaW5kZXggNWVhNzdhNy4u
NWY3YjlmNyAxMDA2NDQNCj4gPiAtLS0gYS9kcml2ZXJzL2lpby90ZW1wZXJhdHVyZS9LY29uZmln
DQo+ID4gKysrIGIvZHJpdmVycy9paW8vdGVtcGVyYXR1cmUvS2NvbmZpZw0KPiA+IEBAIC0xOSw2
ICsxOSwyMCBAQCBjb25maWcgTUFYSU1fVEhFUk1PQ09VUExFDQo+ID4gwqAJwqDCoFRoaXMgZHJp
dmVyIGNhbiBhbHNvIGJlIGJ1aWx0IGFzIGEgbW9kdWxlLiBJZiBzbywgdGhlDQo+ID4gbW9kdWxl
IHdpbGwNCj4gPiDCoAnCoMKgYmUgY2FsbGVkIG1heGltX3RoZXJtb2NvdXBsZS4NCj4gPiDCoA0K
PiA+ICtjb25maWcgSElEX1NFTlNPUl9URU1QDQo+ID4gKwl0cmlzdGF0ZSAiSElEIEVudmlyb25t
ZW50YWwgdGVtcGVyYXR1cmUgc2Vuc29yIg0KPiA+ICsJZGVwZW5kcyBvbiBISURfU0VOU09SX0hV
Qg0KPiA+ICsJc2VsZWN0IElJT19CVUZGRVINCj4gPiArCXNlbGVjdCBJSU9fVFJJR0dFUkVEX0JV
RkZFUg0KPiA+ICsJc2VsZWN0IEhJRF9TRU5TT1JfSUlPX0NPTU1PTg0KPiA+ICsJc2VsZWN0IEhJ
RF9TRU5TT1JfSUlPX1RSSUdHRVINCj4gPiArCWhlbHANCj4gPiArCcKgwqBTYXkgeWVzIGhlcmUg
dG8gYnVpbGQgc3VwcG9ydCBmb3IgdGhlIEhJRCBTRU5TT1INCj4gPiArCcKgwqB0ZW1wZXJhdHVy
ZSBkcml2ZXINCj4gPiArDQo+ID4gKwnCoMKgVG8gY29tcGlsZSB0aGlzIGRyaXZlciBhcyBhIG1v
ZHVsZSwgY2hvb3NlIE0gaGVyZTogdGhlDQo+ID4gbW9kdWxlDQo+ID4gKwnCoMKgd2lsbCBiZSBj
YWxsZWQgaGlkLXNlbnNvci10ZW1wZXJhdHVyZS4NCj4gPiArDQo+ID4gwqBjb25maWcgTUxYOTA2
MTQNCj4gPiDCoAl0cmlzdGF0ZSAiTUxYOTA2MTQgY29udGFjdC1sZXNzIGluZnJhcmVkIHNlbnNv
ciINCj4gPiDCoAlkZXBlbmRzIG9uIEkyQw0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2lpby90
ZW1wZXJhdHVyZS9NYWtlZmlsZQ0KPiA+IGIvZHJpdmVycy9paW8vdGVtcGVyYXR1cmUvTWFrZWZp
bGUNCj4gPiBpbmRleCA3OGMzZGUwLi5lMTU3ZWRhIDEwMDY0NA0KPiA+IC0tLSBhL2RyaXZlcnMv
aWlvL3RlbXBlcmF0dXJlL01ha2VmaWxlDQo+ID4gKysrIGIvZHJpdmVycy9paW8vdGVtcGVyYXR1
cmUvTWFrZWZpbGUNCj4gPiBAQCAtNywzICs3LDQgQEAgb2JqLSQoQ09ORklHX01MWDkwNjE0KSAr
PSBtbHg5MDYxNC5vDQo+ID4gwqBvYmotJChDT05GSUdfVE1QMDA2KSArPSB0bXAwMDYubw0KPiA+
IMKgb2JqLSQoQ09ORklHX1RTWVMwMSkgKz0gdHN5czAxLm8NCj4gPiDCoG9iai0kKENPTkZJR19U
U1lTMDJEKSArPSB0c3lzMDJkLm8NCj4gPiArb2JqLSQoQ09ORklHX0hJRF9TRU5TT1JfVEVNUCnC
oMKgwqArPSBoaWQtc2Vuc29yLXRlbXBlcmF0dXJlLm8NCj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVy
cy9paW8vdGVtcGVyYXR1cmUvaGlkLXNlbnNvci10ZW1wZXJhdHVyZS5jDQo+ID4gYi9kcml2ZXJz
L2lpby90ZW1wZXJhdHVyZS9oaWQtc2Vuc29yLXRlbXBlcmF0dXJlLmMNCj4gPiBuZXcgZmlsZSBt
b2RlIDEwMDY0NA0KPiA+IGluZGV4IDAwMDAwMDAuLjg0YWJkMmQNCj4gPiAtLS0gL2Rldi9udWxs
DQo+ID4gKysrIGIvZHJpdmVycy9paW8vdGVtcGVyYXR1cmUvaGlkLXNlbnNvci10ZW1wZXJhdHVy
ZS5jDQo+ID4gQEAgLTAsMCArMSwzMjYgQEANCj4gPiArLyogSElEIFNlbnNvcnMgRHJpdmVyDQo+
ID4gKyAqIENvcHlyaWdodCAoYykgMjAxNywgSW50ZWwgQ29ycG9yYXRpb24uDQo+ID4gKyAqDQo+
ID4gKyAqIFRoaXMgcHJvZ3JhbSBpcyBmcmVlIHNvZnR3YXJlOyB5b3UgY2FuIHJlZGlzdHJpYnV0
ZSBpdCBhbmQvb3INCj4gPiBtb2RpZnkgaXQNCj4gPiArICogdW5kZXIgdGhlIHRlcm1zIGFuZCBj
b25kaXRpb25zIG9mIHRoZSBHTlUgR2VuZXJhbCBQdWJsaWMNCj4gPiBMaWNlbnNlLA0KPiA+ICsg
KiB2ZXJzaW9uIDIsIGFzIHB1Ymxpc2hlZCBieSB0aGUgRnJlZSBTb2Z0d2FyZSBGb3VuZGF0aW9u
Lg0KPiA+ICsgKg0KPiA+ICsgKiBUaGlzIHByb2dyYW0gaXMgZGlzdHJpYnV0ZWQgaW4gdGhlIGhv
cGUgaXQgd2lsbCBiZSB1c2VmdWwsIGJ1dA0KPiA+IFdJVEhPVVQNCj4gPiArICogQU5ZIFdBUlJB
TlRZOyB3aXRob3V0IGV2ZW4gdGhlIGltcGxpZWQgd2FycmFudHkgb2YNCj4gPiBNRVJDSEFOVEFC
SUxJVFkgb3INCj4gPiArICogRklUTkVTUyBGT1IgQSBQQVJUSUNVTEFSIFBVUlBPU0UuwqDCoFNl
ZSB0aGUgR05VIEdlbmVyYWwgUHVibGljDQo+ID4gTGljZW5zZSBmb3INCj4gPiArICogbW9yZSBk
ZXRhaWxzLg0KPiA+ICsgKg0KPiA+ICsgKiBZb3Ugc2hvdWxkIGhhdmUgcmVjZWl2ZWQgYSBjb3B5
IG9mIHRoZSBHTlUgR2VuZXJhbCBQdWJsaWMNCj4gPiBMaWNlbnNlIGFsb25nIHdpdGgNCj4gPiAr
ICogdGhpcyBwcm9ncmFtLg0KPiA+ICsgKg0KPiA+ICsgKi8NCj4gPiArI2luY2x1ZGUgPGxpbnV4
L2RldmljZS5oPg0KPiA+ICsjaW5jbHVkZSA8bGludXgvaGlkLXNlbnNvci1odWIuaD4NCj4gPiAr
I2luY2x1ZGUgPGxpbnV4L2lpby9idWZmZXIuaD4NCj4gPiArI2luY2x1ZGUgPGxpbnV4L2lpby9p
aW8uaD4NCj4gPiArI2luY2x1ZGUgPGxpbnV4L2lpby90cmlnZ2VyZWRfYnVmZmVyLmg+DQo+ID4g
KyNpbmNsdWRlIDxsaW51eC9paW8vdHJpZ2dlcl9jb25zdW1lci5oPg0KPiA+ICsjaW5jbHVkZSA8
bGludXgvbW9kdWxlLmg+DQo+ID4gKyNpbmNsdWRlIDxsaW51eC9wbGF0Zm9ybV9kZXZpY2UuaD4N
Cj4gPiArI2luY2x1ZGUgIi4uL2NvbW1vbi9oaWQtc2Vuc29ycy9oaWQtc2Vuc29yLXRyaWdnZXIu
aCINCj4gPiArDQo+ID4gK3N0cnVjdCB0ZW1wZXJhdHVyZV9zdGF0ZSB7DQo+ID4gKwlzdHJ1Y3Qg
aGlkX3NlbnNvcl9jb21tb24gY29tbW9uX2F0dHJpYnV0ZXM7DQo+ID4gKwlzdHJ1Y3QgaGlkX3Nl
bnNvcl9odWJfYXR0cmlidXRlX2luZm8gdGVtcGVyYXR1cmVfYXR0cjsNCj4gPiArCXMzMiB0ZW1w
ZXJhdHVyZV9kYXRhOw0KPiA+ICsJaW50IHNjYWxlX3ByZV9kZWNtbDsNCj4gPiArCWludCBzY2Fs
ZV9wb3N0X2RlY21sOw0KPiA+ICsJaW50IHNjYWxlX3ByZWNpc2lvbjsNCj4gPiArCWludCB2YWx1
ZV9vZmZzZXQ7DQo+ID4gK307DQo+ID4gKw0KPiA+ICsvKiBDaGFubmVsIGRlZmluaXRpb25zICov
DQo+ID4gK3N0YXRpYyBjb25zdCBzdHJ1Y3QgaWlvX2NoYW5fc3BlYyB0ZW1wZXJhdHVyZV9jaGFu
bmVsc1tdID0gew0KPiA+ICsJew0KPiA+ICsJCS50eXBlID0gSUlPX1RFTVAsDQo+ID4gKwkJLmlu
Zm9fbWFza19zZXBhcmF0ZSA9IEJJVChJSU9fQ0hBTl9JTkZPX1JBVyksDQo+ID4gKwkJLmluZm9f
bWFza19zaGFyZWRfYnlfdHlwZSA9DQo+ID4gQklUKElJT19DSEFOX0lORk9fT0ZGU0VUKSB8DQo+
ID4gKwkJCUJJVChJSU9fQ0hBTl9JTkZPX1NDQUxFKSB8DQo+ID4gKwkJCUJJVChJSU9fQ0hBTl9J
TkZPX1NBTVBfRlJFUSkgfA0KPiA+ICsJCQlCSVQoSUlPX0NIQU5fSU5GT19IWVNURVJFU0lTKSwN
Cj4gPiArCQkuc2Nhbl9pbmRleCA9IDAsDQo+ID4gKwl9DQo+ID4gK307DQo+ID4gKw0KPiA+ICsv
KiBBZGp1c3QgY2hhbm5lbCByZWFsIGJpdHMgYmFzZWQgb24gcmVwb3J0IGRlc2NyaXB0b3IgKi8N
Cj4gPiArc3RhdGljIHZvaWQgdGVtcGVyYXR1cmVfYWRqdXN0X2NoYW5uZWxfYml0X21hc2soc3Ry
dWN0DQo+ID4gaWlvX2NoYW5fc3BlYyAqY2hhbm5lbHMsDQo+ID4gKwkJCQkJaW50IGNoYW5uZWws
IGludCBzaXplKQ0KPiA+ICt7DQo+ID4gKwljaGFubmVsc1tjaGFubmVsXS5zY2FuX3R5cGUuc2ln
biA9ICdzJzsNCj4gPiArCS8qIFJlYWwgc3RvcmFnZSBiaXRzIHdpbGwgY2hhbmdlIGJhc2VkIG9u
IHRoZSByZXBvcnQgZGVzYy4NCj4gPiAqLw0KPiA+ICsJY2hhbm5lbHNbY2hhbm5lbF0uc2Nhbl90
eXBlLnJlYWxiaXRzID0gc2l6ZSAqIDg7DQo+ID4gKwkvKiBNYXhpbXVtIHNpemUgb2YgYSBzYW1w
bGUgdG8gY2FwdHVyZSBpcyBzMzIgKi8NCj4gPiArCWNoYW5uZWxzW2NoYW5uZWxdLnNjYW5fdHlw
ZS5zdG9yYWdlYml0cyA9IHNpemVvZihzMzIpICogODsNCj4gPiArfQ0KPiA+ICsNCj4gPiArLyog
Q2hhbm5lbCByZWFkX3JhdyBoYW5kbGVyICovDQo+ID4gK3N0YXRpYyBpbnQgdGVtcGVyYXR1cmVf
cmVhZF9yYXcoc3RydWN0IGlpb19kZXYgKmluZGlvX2RldiwNCj4gPiArCQkJCXN0cnVjdCBpaW9f
Y2hhbl9zcGVjIGNvbnN0ICpjaGFuLA0KPiA+ICsJCQkJaW50ICp2YWwsIGludCAqdmFsMiwgbG9u
ZyBtYXNrKQ0KPiA+ICt7DQo+ID4gKwlzdHJ1Y3QgdGVtcGVyYXR1cmVfc3RhdGUgKnRlbXBlcmF0
dXJlX3N0YXRlID0NCj4gPiBpaW9fcHJpdihpbmRpb19kZXYpOw0KPiA+ICsJaW50IHJldDsNCj4g
PiArDQo+ID4gKwlzd2l0Y2ggKG1hc2spIHsNCj4gPiArCWNhc2UgSUlPX0NIQU5fSU5GT19SQVc6
DQo+ID4gKwkJaWYgKGNoYW4tPnR5cGUgPT0gSUlPX1RFTVApIHsNCj4gPiArCQkJaGlkX3NlbnNv
cl9wb3dlcl9zdGF0ZSgNCj4gPiArCQkJCSZ0ZW1wZXJhdHVyZV9zdGF0ZS0NCj4gPiA+Y29tbW9u
X2F0dHJpYnV0ZXMsIHRydWUpOw0KPiA+ICsJCQkqdmFsID0NCj4gPiBzZW5zb3JfaHViX2lucHV0
X2F0dHJfZ2V0X3Jhd192YWx1ZSgNCj4gPiArCQkJCXRlbXBlcmF0dXJlX3N0YXRlLQ0KPiA+ID5j
b21tb25fYXR0cmlidXRlcy5oc2RldiwNCj4gPiArCQkJCUhJRF9VU0FHRV9TRU5TT1JfVEVNUEVS
QVRVUkUsDQo+ID4gKwkJCQlISURfVVNBR0VfU0VOU09SX0RBVEFfRU5WSVJPTk1FTlRBDQo+ID4g
TF9URU1QRVJBVFVSRSwNCj4gPiArCQkJCXRlbXBlcmF0dXJlX3N0YXRlLQ0KPiA+ID50ZW1wZXJh
dHVyZV9hdHRyLnJlcG9ydF9pZCwNCj4gPiArCQkJCVNFTlNPUl9IVUJfU1lOQyk7DQo+ID4gKwkJ
CWhpZF9zZW5zb3JfcG93ZXJfc3RhdGUoDQo+ID4gKwkJCQkJJnRlbXBlcmF0dXJlX3N0YXRlLQ0K
PiA+ID5jb21tb25fYXR0cmlidXRlcywNCj4gPiArCQkJCQlmYWxzZSk7DQo+ID4gKwkJfQ0KPiAN
Cj4gVGhpcyBpcyBhbiBvZGRpdHkuwqDCoElmIHlvdSBnZXQgaGVyZSB3aXRob3V0IGNoYW4tPnR5
cGUgPT0gSUlPX1RFTVANCj4gKHdoaWNoIHlvdSBjYW4ndCwgYnV0IHlvdXIgY29kZSBpcyBraW5k
IG9mIGFzc3VtaW5nIHlvdSBjYW4pDQo+IHRoZW4geW91IGFyZSByZXR1cm5pbmcgdGhhdCB0aGUg
ZGF0YSBpcyB2YWxpZCB3aXRob3V0IHNldHRpbmcgaXQgdG8NCj4gYW55dGhpbmcuDQo+IA0KPiBJ
J2QgZmxpcCB0aGUgbG9naWMNCj4gaWYgKGNoYW4tPnR5cGUgIT0gSUlPX1RFTVApIHsNCj4gwqDC
oMKgcmV0dXJuIC1FSU5WQUw7DQo+IH0NCj4gLi4uIG5vcm1hbCBjb2RlIGZsb3cgLi4uDQo+ID4g
KwkJcmV0ID0gSUlPX1ZBTF9JTlQ7DQo+IA0KPiBUaGVyZSBpcyBub3RoaW5nIG90aGVyIHRoYW4g
YSByZXR1cm4gdG8gYmUgZG9uZSBhZnRlciB0aGlzIHNvIGp1c3QNCj4gZG8gaXQgZGlyZWN0bHkg
aGVyZSBpbnN0ZWFkLsKgwqBCYXNpYyBydWxlIG9mIHRodW1iIGluIGtlcm5lbCBjb2RlIGlzDQo+
IHJldHVybiBhcyBlYXJseSBhcyBwb3NzaWJsZSB3aGVuIHRoZXJlIGlzIG5vIHVud2luZGluZyB0
byBiZSBkb25lLg0KPiBTYW1lIGluIGFsbCB0aGUgYnJhbmNoZXMgb2YgdGhpcyBzd2l0Y2ggc3Rh
dGVtZW50IHBlbGFzZS4NCj4gPiArCQlicmVhazsNCj4gPiArCWNhc2UgSUlPX0NIQU5fSU5GT19T
Q0FMRToNCj4gPiArCQkqdmFsID0gdGVtcGVyYXR1cmVfc3RhdGUtPnNjYWxlX3ByZV9kZWNtbDsN
Cj4gPiArCQkqdmFsMiA9IHRlbXBlcmF0dXJlX3N0YXRlLT5zY2FsZV9wb3N0X2RlY21sOw0KPiA+
ICsJCXJldCA9IHRlbXBlcmF0dXJlX3N0YXRlLT5zY2FsZV9wcmVjaXNpb247DQo+ID4gKwkJYnJl
YWs7DQo+ID4gKwljYXNlIElJT19DSEFOX0lORk9fT0ZGU0VUOg0KPiA+ICsJCSp2YWwgPSB0ZW1w
ZXJhdHVyZV9zdGF0ZS0+dmFsdWVfb2Zmc2V0Ow0KPiA+ICsJCXJldCA9IElJT19WQUxfSU5UOw0K
PiA+ICsJCWJyZWFrOw0KPiA+ICsJY2FzZSBJSU9fQ0hBTl9JTkZPX1NBTVBfRlJFUToNCj4gPiAr
CQlyZXQgPSBoaWRfc2Vuc29yX3JlYWRfc2FtcF9mcmVxX3ZhbHVlKA0KPiA+ICsJCQkJJnRlbXBl
cmF0dXJlX3N0YXRlLQ0KPiA+ID5jb21tb25fYXR0cmlidXRlcywNCj4gPiArCQkJCXZhbCwgdmFs
Mik7DQo+ID4gKwkJYnJlYWs7DQo+ID4gKwljYXNlIElJT19DSEFOX0lORk9fSFlTVEVSRVNJUzoN
Cj4gPiArCQlyZXQgPSBoaWRfc2Vuc29yX3JlYWRfcmF3X2h5c3RfdmFsdWUoDQo+ID4gKwkJCQkm
dGVtcGVyYXR1cmVfc3RhdGUtDQo+ID4gPmNvbW1vbl9hdHRyaWJ1dGVzLA0KPiA+ICsJCQkJdmFs
LCB2YWwyKTsNCj4gPiArCQlicmVhazsNCj4gPiArCWRlZmF1bHQ6DQo+ID4gKwkJcmV0ID0gLUVJ
TlZBTDsNCj4gPiArCX0NCj4gPiArDQo+ID4gKwlyZXR1cm4gcmV0Ow0KPiA+ICt9DQo+ID4gKw0K
PiA+ICsvKiBDaGFubmVsIHdyaXRlX3JhdyBoYW5kbGVyICovDQo+IA0KPiBQZXJoYXBzIGRyb3Ag
dGhlIG1vcmUgb2J2aW91cyBjb21tZW50cyBhcyBub3QgbmVjZXNzYXJpbHkgcHJvdmlkaW5nDQo+
IGFueSBhZGRpdGlvbmFsIGluZm8uwqDCoEkgZG9uJ3QgcmVhbGx5IG1pbmQgaWYgeW91IGZlZWwg
dGhleSBhcmUgdXNlZnVsDQo+IGhvd2V2ZXIgYW5kIHdhbnQgdG8ga2VlcCB0aGVtLg0KPiA+ICtz
dGF0aWMgaW50IHRlbXBlcmF0dXJlX3dyaXRlX3JhdyhzdHJ1Y3QgaWlvX2RldiAqaW5kaW9fZGV2
LA0KPiA+ICsJCQkJc3RydWN0IGlpb19jaGFuX3NwZWMgY29uc3QgKmNoYW4sDQo+ID4gKwkJCQlp
bnQgdmFsLCBpbnQgdmFsMiwgbG9uZyBtYXNrKQ0KPiA+ICt7DQo+ID4gKwlzdHJ1Y3QgdGVtcGVy
YXR1cmVfc3RhdGUgKnRlbXBlcmF0dXJlX3N0YXRlID0NCj4gPiBpaW9fcHJpdihpbmRpb19kZXYp
Ow0KPiA+ICsJaW50IHJldDsNCj4gPiArDQo+ID4gKwlzd2l0Y2ggKG1hc2spIHsNCj4gPiArCWNh
c2UgSUlPX0NIQU5fSU5GT19TQU1QX0ZSRVE6DQo+ID4gKwkJcmV0ID0gaGlkX3NlbnNvcl93cml0
ZV9zYW1wX2ZyZXFfdmFsdWUoDQo+ID4gKwkJCQkmdGVtcGVyYXR1cmVfc3RhdGUtDQo+ID4gPmNv
bW1vbl9hdHRyaWJ1dGVzLCB2YWwsDQo+ID4gKwkJCQl2YWwyKTsNCj4gDQo+IEFzIHdpdGggdGhl
IHJlYWRfcmF3IGFib3ZlLCByZXR1cm4gZGlyZWN0bHkgd2l0aGluIHRoZXNlIHN0YXRlbWVudHMN
Cj4gcmF0aGVyIHRoYW4NCj4gaGF2aW5nIHRvIGJvdGhlciB3aXRoIGJyZWFraW5nIG91dCBvZiB0
aGUgc3dpdGNoIGFuZCB0aGVuIGRvaW5nIGl0Lg0KPiBUaWRpZXIgY29kZSB0aGF0IHdheS4NCj4g
DQo+IHJldHVybiBoaWRfc2Vuc29yLi4uLg0KPiA+ICsJCWJyZWFrOw0KPiA+ICsJY2FzZSBJSU9f
Q0hBTl9JTkZPX0hZU1RFUkVTSVM6DQo+ID4gKwkJcmV0ID0gaGlkX3NlbnNvcl93cml0ZV9yYXdf
aHlzdF92YWx1ZSgNCj4gPiArCQkJCSZ0ZW1wZXJhdHVyZV9zdGF0ZS0NCj4gPiA+Y29tbW9uX2F0
dHJpYnV0ZXMsIHZhbCwNCj4gPiArCQkJCXZhbDIpOw0KPiA+ICsJCWJyZWFrOw0KPiA+ICsJZGVm
YXVsdDoNCj4gPiArCQlyZXQgPSAtRUlOVkFMOw0KPiA+ICsJfQ0KPiA+ICsNCj4gPiArCXJldHVy
biByZXQ7DQo+ID4gK30NCj4gPiArDQo+ID4gK3N0YXRpYyBjb25zdCBzdHJ1Y3QgaWlvX2luZm8g
dGVtcGVyYXR1cmVfaW5mbyA9IHsNCj4gPiArCS5kcml2ZXJfbW9kdWxlID0gVEhJU19NT0RVTEUs
DQo+ID4gKwkucmVhZF9yYXcgPSAmdGVtcGVyYXR1cmVfcmVhZF9yYXcsDQo+ID4gKwkud3JpdGVf
cmF3ID0gJnRlbXBlcmF0dXJlX3dyaXRlX3JhdywNCj4gPiArfTsNCj4gPiArDQo+ID4gKy8qIENh
bGxiYWNrIGhhbmRsZXIgdG8gc2VuZCBldmVudCBhZnRlciBhbGwgc2FtcGxlcyBhcmUgcmVjZWl2
ZWQNCj4gPiBhbmQgY2FwdHVyZWQgKi8NCj4gPiArc3RhdGljIGludCB0ZW1wZXJhdHVyZV9wcm9j
X2V2ZW50KHN0cnVjdCBoaWRfc2Vuc29yX2h1Yl9kZXZpY2UNCj4gPiAqaHNkZXYsDQo+ID4gKwkJ
CQl1bnNpZ25lZCBpbnQgdXNhZ2VfaWQsIHZvaWQgKnBkZXYpDQo+ID4gK3sNCj4gPiArCXN0cnVj
dCBpaW9fZGV2ICppbmRpb19kZXYgPSBwbGF0Zm9ybV9nZXRfZHJ2ZGF0YShwZGV2KTsNCj4gPiAr
CXN0cnVjdCB0ZW1wZXJhdHVyZV9zdGF0ZSAqdGVtcGVyYXR1cmVfc3RhdGUgPQ0KPiA+IGlpb19w
cml2KGluZGlvX2Rldik7DQo+ID4gKw0KPiA+ICsJaWYgKGF0b21pY19yZWFkKCZ0ZW1wZXJhdHVy
ZV9zdGF0ZS0NCj4gPiA+Y29tbW9uX2F0dHJpYnV0ZXMuZGF0YV9yZWFkeSkpDQo+ID4gKwkJaWlv
X3B1c2hfdG9fYnVmZmVycyhpbmRpb19kZXYsDQo+ID4gKwkJCQkJJnRlbXBlcmF0dXJlX3N0YXRl
LQ0KPiA+ID50ZW1wZXJhdHVyZV9kYXRhKTsNCj4gPiArCXJldHVybiAwOw0KPiA+ICt9DQo+ID4g
Kw0KPiA+ICsvKiBDYXB0dXJlIHNhbXBsZXMgaW4gbG9jYWwgc3RvcmFnZSAqLw0KPiA+ICtzdGF0
aWMgaW50IHRlbXBlcmF0dXJlX2NhcHR1cmVfc2FtcGxlKHN0cnVjdCBoaWRfc2Vuc29yX2h1Yl9k
ZXZpY2UNCj4gPiAqaHNkZXYsDQo+ID4gKwkJCQl1bnNpZ25lZCBpbnQgdXNhZ2VfaWQsIHNpemVf
dA0KPiA+IHJhd19sZW4sDQo+ID4gKwkJCQljaGFyICpyYXdfZGF0YSwgdm9pZCAqcGRldikNCj4g
PiArew0KPiA+ICsJc3RydWN0IGlpb19kZXYgKmluZGlvX2RldiA9IHBsYXRmb3JtX2dldF9kcnZk
YXRhKHBkZXYpOw0KPiA+ICsJc3RydWN0IHRlbXBlcmF0dXJlX3N0YXRlICp0ZW1wZXJhdHVyZV9z
dGF0ZSA9DQo+ID4gaWlvX3ByaXYoaW5kaW9fZGV2KTsNCj4gPiArDQo+ID4gKwlzd2l0Y2ggKHVz
YWdlX2lkKSB7DQo+ID4gKwljYXNlIEhJRF9VU0FHRV9TRU5TT1JfREFUQV9FTlZJUk9OTUVOVEFM
X1RFTVBFUkFUVVJFOg0KPiA+ICsJCXRlbXBlcmF0dXJlX3N0YXRlLT50ZW1wZXJhdHVyZV9kYXRh
ID0gKihzMzINCj4gPiAqKXJhd19kYXRhOw0KPiA+ICsJCXJldHVybiAwOw0KPiA+ICsJZGVmYXVs
dDoNCj4gPiArCQlyZXR1cm4gLUVJTlZBTDsNCj4gPiArCX0NCj4gPiArfQ0KPiA+ICsNCj4gPiAr
LyogUGFyc2UgcmVwb3J0IHdoaWNoIGlzIHNwZWNpZmljIHRvIGFuIHVzYWdlIGlkKi8NCj4gPiAr
c3RhdGljIGludCB0ZW1wZXJhdHVyZV9wYXJzZV9yZXBvcnQoc3RydWN0IHBsYXRmb3JtX2Rldmlj
ZSAqcGRldiwNCj4gPiArCQkJCXN0cnVjdCBoaWRfc2Vuc29yX2h1Yl9kZXZpY2UNCj4gPiAqaHNk
ZXYsDQo+ID4gKwkJCQlzdHJ1Y3QgaWlvX2NoYW5fc3BlYyAqY2hhbm5lbHMsDQo+ID4gKwkJCQl1
bnNpZ25lZCBpbnQgdXNhZ2VfaWQsDQo+ID4gKwkJCQlzdHJ1Y3QgdGVtcGVyYXR1cmVfc3RhdGUg
KnN0KQ0KPiA+ICt7DQo+ID4gKwlpbnQgcmV0Ow0KPiA+ICsNCj4gPiArCXJldCA9IHNlbnNvcl9o
dWJfaW5wdXRfZ2V0X2F0dHJpYnV0ZV9pbmZvKGhzZGV2LA0KPiA+IEhJRF9JTlBVVF9SRVBPUlQs
DQo+ID4gKwkJCXVzYWdlX2lkLA0KPiA+ICsJCQlISURfVVNBR0VfU0VOU09SX0RBVEFfRU5WSVJP
Tk1FTlRBTF9URU1QRVINCj4gPiBBVFVSRSwNCj4gPiArCQkJJnN0LT50ZW1wZXJhdHVyZV9hdHRy
KTsNCj4gPiArCWlmIChyZXQgPCAwKQ0KPiA+ICsJCXJldHVybiByZXQ7DQo+ID4gKw0KPiA+ICsJ
dGVtcGVyYXR1cmVfYWRqdXN0X2NoYW5uZWxfYml0X21hc2soY2hhbm5lbHMsIDAsDQo+ID4gKwkJ
CQkJc3QtDQo+ID4gPnRlbXBlcmF0dXJlX2F0dHIuc2l6ZSk7DQo+ID4gKw0KPiA+ICsJc3QtPnNj
YWxlX3ByZWNpc2lvbiA9IGhpZF9zZW5zb3JfZm9ybWF0X3NjYWxlKA0KPiA+ICsJCQkJSElEX1VT
QUdFX1NFTlNPUl9URU1QRVJBVFVSRSwNCj4gPiArCQkJCSZzdC0+dGVtcGVyYXR1cmVfYXR0ciwN
Cj4gPiArCQkJCSZzdC0+c2NhbGVfcHJlX2RlY21sLCAmc3QtDQo+ID4gPnNjYWxlX3Bvc3RfZGVj
bWwpOw0KPiA+ICsNCj4gPiArCS8qIFNldCBTZW5zaXRpdml0eSBmaWVsZCBpZHMsIHdoZW4gdGhl
cmUgaXMgbm8gaW5kaXZpZHVhbA0KPiA+IG1vZGlmaWVyICovDQo+ID4gKwlpZiAoc3QtPmNvbW1v
bl9hdHRyaWJ1dGVzLnNlbnNpdGl2aXR5LmluZGV4IDwgMCkNCj4gPiArCQlzZW5zb3JfaHViX2lu
cHV0X2dldF9hdHRyaWJ1dGVfaW5mbyhoc2RldiwNCj4gPiArCQkJSElEX0ZFQVRVUkVfUkVQT1JU
LCB1c2FnZV9pZCwNCj4gPiArCQkJSElEX1VTQUdFX1NFTlNPUl9EQVRBX01PRF9DSEFOR0VfU0VO
U0lUSVZJDQo+ID4gVFlfQUJTIHwNCj4gPiArCQkJSElEX1VTQUdFX1NFTlNPUl9EQVRBX0VOVklS
T05NRU5UQUxfVEVNUEVSDQo+ID4gQVRVUkUsDQo+ID4gKwkJCSZzdC0+Y29tbW9uX2F0dHJpYnV0
ZXMuc2Vuc2l0aXZpdHkpOw0KPiA+ICsNCj4gPiArCXJldHVybiByZXQ7DQo+ID4gK30NCj4gPiAr
DQo+ID4gK3N0YXRpYyBzdHJ1Y3QgaGlkX3NlbnNvcl9odWJfY2FsbGJhY2tzIHRlbXBlcmF0dXJl
X2NhbGxiYWNrcyA9IHsNCj4gPiArCS5zZW5kX2V2ZW50ID0gJnRlbXBlcmF0dXJlX3Byb2NfZXZl
bnQsDQo+ID4gKwkuY2FwdHVyZV9zYW1wbGUgPSAmdGVtcGVyYXR1cmVfY2FwdHVyZV9zYW1wbGUs
DQo+ID4gK307DQo+ID4gKw0KPiA+ICsvKiBGdW5jdGlvbiB0byBpbml0aWFsaXplIHRoZSBwcm9j
ZXNzaW5nIGZvciB1c2FnZSBpZCAqLw0KPiA+ICtzdGF0aWMgaW50IGhpZF90ZW1wZXJhdHVyZV9w
cm9iZShzdHJ1Y3QgcGxhdGZvcm1fZGV2aWNlICpwZGV2KQ0KPiA+ICt7DQo+ID4gKwlzdGF0aWMg
Y29uc3QgY2hhciAqbmFtZSA9ICJ0ZW1wZXJhdHVyZSI7DQo+ID4gKwlzdHJ1Y3QgaWlvX2RldiAq
aW5kaW9fZGV2Ow0KPiA+ICsJc3RydWN0IHRlbXBlcmF0dXJlX3N0YXRlICp0ZW1wZXJhdHVyZV9z
dGF0ZTsNCj4gDQo+IE1heWJlIHNob3J0ZW4gdGhlIHZhcmlhYmxlIG5hbWUgdG8gbGVhZCB0byBz
b21lIHNob3J0ZXIgbGluZXMgbGF0ZXI/DQo+IHRlbXBfc3QgZm9yIGV4YW1wbGUgd291bGQgSSB0
aGluayBjb252ZXkgZW5vdWdoIG1lYW5pbmcuDQo+IA0KPiA+ICsJc3RydWN0IGhpZF9zZW5zb3Jf
aHViX2RldmljZSAqaHNkZXYgPSBwZGV2LQ0KPiA+ID5kZXYucGxhdGZvcm1fZGF0YTsNCj4gPiAr
CWludCByZXQ7DQo+ID4gKw0KPiA+ICsJaW5kaW9fZGV2ID0gZGV2bV9paW9fZGV2aWNlX2FsbG9j
KCZwZGV2LT5kZXYsDQo+ID4gKwkJCQlzaXplb2Yoc3RydWN0IHRlbXBlcmF0dXJlX3N0YXRlKSk7
DQo+IA0KPiBFdmVyIHNvIHNsaWdodCBwcmVmZXJlbmNlIGZvciBzaXplb2YoKnRlbXBlcmF0dXJl
X3N0YXRlKQ0KPiANCj4gPiArCWlmICghaW5kaW9fZGV2KQ0KPiA+ICsJCXJldHVybiAtRU5PTUVN
Ow0KPiA+ICsNCj4gPiArCXRlbXBlcmF0dXJlX3N0YXRlID0gaWlvX3ByaXYoaW5kaW9fZGV2KTsN
Cj4gPiArCXRlbXBlcmF0dXJlX3N0YXRlLT5jb21tb25fYXR0cmlidXRlcy5oc2RldiA9IGhzZGV2
Ow0KPiA+ICsJdGVtcGVyYXR1cmVfc3RhdGUtPmNvbW1vbl9hdHRyaWJ1dGVzLnBkZXYgPSBwZGV2
Ow0KPiA+ICsNCj4gPiArCXJldCA9IGhpZF9zZW5zb3JfcGFyc2VfY29tbW9uX2F0dHJpYnV0ZXMo
aHNkZXYsDQo+ID4gKwkJCQkJSElEX1VTQUdFX1NFTlNPUl9URU1QRVJBVFUNCj4gPiBSRSwNCj4g
PiArCQkJCQkmdGVtcGVyYXR1cmVfc3RhdGUtDQo+ID4gPmNvbW1vbl9hdHRyaWJ1dGVzKTsNCj4g
PiArCWlmIChyZXQpDQo+ID4gKwkJcmV0dXJuIHJldDsNCj4gPiArDQo+ID4gKwlpbmRpb19kZXYt
PmNoYW5uZWxzID0gZGV2bV9rbWVtZHVwKCZpbmRpb19kZXYtPmRldiwNCj4gDQo+IG1heWJlIHJl
bmFtZSBhcyB0ZW1wX2NoYW5zIGFzIHZlcmJvc2l0eSBub3QgYWRkaW5nIG11Y2ggaGVyZS4NCj4g
PiArCQkJCQl0ZW1wZXJhdHVyZV9jaGFubmVscywNCj4gPiArCQkJCQlzaXplb2YodGVtcGVyYXR1
cmVfY2hhbm5lbA0KPiA+IHMpLA0KPiA+ICsJCQkJCUdGUF9LRVJORUwpOw0KPiA+ICsJaWYgKCFp
bmRpb19kZXYtPmNoYW5uZWxzKQ0KPiA+ICsJCXJldHVybiAtRU5PTUVNOw0KPiA+ICsNCj4gPiAr
CXJldCA9IHRlbXBlcmF0dXJlX3BhcnNlX3JlcG9ydChwZGV2LCBoc2RldiwNCj4gPiArCQkJCShz
dHJ1Y3QgaWlvX2NoYW5fc3BlYyAqKWluZGlvX2Rldi0NCj4gPiA+Y2hhbm5lbHMsDQo+IA0KPiBU
aGlzIGlzIGNhc3RpbmcgYXdheSBhIGNvbnN0LCB3aGljaCBJIGRvbid0IHBhcnRpY3VsYXJseSBs
aWtlLg0KPiBJJ2QgcHJlZmVyIHRoYXQgeW91IHVzZWQgYSBsb2NhbCB2YXJpYWJsZSwgZ290IGV2
ZXJ0aGluZyBzZXR1cCBhcyB5b3UNCj4gbGlrZQ0KPiBhbmQgdGhlbiBhc3NpZ25lZCBpbmRpb19k
ZXYtPmNoYW5uZWxzIHRvIGl0LsKgwqBTZW1hbnRpY2FsbHkgc2xpZ2h0bHkNCj4gbmljZXIgYXMN
Cj4gYXNzdW1wdGlvbiBpcyB0aGF0IHRoZXkgYXJlIGNvbnN0IGlmIGFjY2Vzc2VkIHZpYSB0aGlz
IGVsZW1lbnQgb2YNCj4gc3RydWN0DQo+IGlpb19kZXYgKGhlcmUgd2Ugb2J2aW91c2x5IGtub3cg
d2UgY2FuIGNoYW5nZSB0aGVtIGJ1dCBpdCdzIHN0aWxsIG5vdA0KPiBuaWNlISkNCj4gDQo+ID4g
KwkJCQlISURfVVNBR0VfU0VOU09SX1RFTVBFUkFUVVJFLA0KPiA+ICsJCQkJdGVtcGVyYXR1cmVf
c3RhdGUpOw0KPiA+ICsJaWYgKHJldCkNCj4gPiArCQlyZXR1cm4gcmV0Ow0KPiA+ICsNCj4gPiAr
CWluZGlvX2Rldi0+bnVtX2NoYW5uZWxzID0NCj4gPiBBUlJBWV9TSVpFKHRlbXBlcmF0dXJlX2No
YW5uZWxzKTsNCj4gPiArCWluZGlvX2Rldi0+ZGV2LnBhcmVudCA9ICZwZGV2LT5kZXY7DQo+ID4g
KwlpbmRpb19kZXYtPmluZm8gPSAmdGVtcGVyYXR1cmVfaW5mbzsNCj4gPiArCWluZGlvX2Rldi0+
bmFtZSA9IG5hbWU7DQo+ID4gKwlpbmRpb19kZXYtPm1vZGVzID0gSU5ESU9fRElSRUNUX01PREU7
DQo+ID4gKw0KPiA+ICsJcmV0ID0gZGV2bV9paW9fdHJpZ2dlcmVkX2J1ZmZlcl9zZXR1cCgmcGRl
di0+ZGV2LA0KPiA+IGluZGlvX2RldiwNCj4gPiArCQkJCQkmaWlvX3BvbGxmdW5jX3N0b3JlX3Rp
bWUsDQo+IA0KPiBPZGRpdHkgaGVyZSBhcyB5b3UgZG9uJ3QgYWN0dWFsbHkgdXNlIHRoZSB0aW1l
c3RhbXAuLi4NCj4gPiArCQkJCQlOVUxMLCBOVUxMKTsNCj4gPiArCWlmIChyZXQpDQo+ID4gKwkJ
cmV0dXJuIHJldDsNCj4gPiArDQo+ID4gKwlhdG9taWNfc2V0KCZ0ZW1wZXJhdHVyZV9zdGF0ZS0N
Cj4gPiA+Y29tbW9uX2F0dHJpYnV0ZXMuZGF0YV9yZWFkeSwgMCk7DQo+ID4gKwlyZXQgPSBoaWRf
c2Vuc29yX3NldHVwX3RyaWdnZXIoaW5kaW9fZGV2LCBuYW1lLA0KPiA+ICsJCQkJJnRlbXBlcmF0
dXJlX3N0YXRlLQ0KPiA+ID5jb21tb25fYXR0cmlidXRlcyk7DQo+ID4gKwlpZiAocmV0KQ0KPiA+
ICsJCXJldHVybiByZXQ7DQo+ID4gKw0KPiA+ICsJcGxhdGZvcm1fc2V0X2RydmRhdGEocGRldiwg
aW5kaW9fZGV2KTsNCj4gPiArDQo+ID4gKwl0ZW1wZXJhdHVyZV9jYWxsYmFja3MucGRldiA9IHBk
ZXY7DQo+ID4gKwlyZXQgPSBzZW5zb3JfaHViX3JlZ2lzdGVyX2NhbGxiYWNrKGhzZGV2LA0KPiA+
IEhJRF9VU0FHRV9TRU5TT1JfVEVNUEVSQVRVUkUsDQo+ID4gKwkJCQkJJnRlbXBlcmF0dXJlX2Nh
bGxiYWNrcyk7DQo+ID4gKwlpZiAocmV0KQ0KPiA+ICsJCWdvdG8gZXJyb3JfcmVtb3ZlX3RyaWdn
ZXI7DQo+ID4gKw0KPiA+ICsJcmV0ID0gZGV2bV9paW9fZGV2aWNlX3JlZ2lzdGVyKGluZGlvX2Rl
di0+ZGV2LnBhcmVudCwNCj4gPiBpbmRpb19kZXYpOw0KPiA+ICsJaWYgKHJldCkNCj4gPiArCQln
b3RvIGVycm9yX3JlbW92ZV9jYWxsYmFjazsNCj4gPiArDQo+ID4gKwlyZXR1cm4gcmV0Ow0KPiA+
ICsNCj4gPiArZXJyb3JfcmVtb3ZlX2NhbGxiYWNrOg0KPiA+ICsJc2Vuc29yX2h1Yl9yZW1vdmVf
Y2FsbGJhY2soaHNkZXYsDQo+ID4gSElEX1VTQUdFX1NFTlNPUl9URU1QRVJBVFVSRSk7DQo+ID4g
K2Vycm9yX3JlbW92ZV90cmlnZ2VyOg0KPiA+ICsJaGlkX3NlbnNvcl9yZW1vdmVfdHJpZ2dlcigm
dGVtcGVyYXR1cmVfc3RhdGUtDQo+ID4gPmNvbW1vbl9hdHRyaWJ1dGVzKTsNCj4gPiArCXJldHVy
biByZXQ7DQo+ID4gK30NCj4gPiArDQo+ID4gKy8qIEZ1bmN0aW9uIHRvIGRlaW5pdGlhbGl6ZSB0
aGUgcHJvY2Vzc2luZyBmb3IgdXNhZ2UgaWQgKi8NCj4gPiArc3RhdGljIGludCBoaWRfdGVtcGVy
YXR1cmVfcmVtb3ZlKHN0cnVjdCBwbGF0Zm9ybV9kZXZpY2UgKnBkZXYpDQo+ID4gK3sNCj4gPiAr
CXN0cnVjdCBoaWRfc2Vuc29yX2h1Yl9kZXZpY2UgKmhzZGV2ID0gcGRldi0NCj4gPiA+ZGV2LnBs
YXRmb3JtX2RhdGE7DQo+IA0KPiBJcyBpdCBqdXN0IG1lIHRoYXQgZmVlbHMgdGhhdCBmb3IgY29u
c2lzdGVuY2UgdGhlcmUgc2hvdWxkIGJlIGENCj4gcGxhdGZvcm1fZ2V0X3BsYXRmb3JtX2RhdGEo
KSBmdW5jdGlvbj8NCj4gQSB3ZWxsLCBjYW4ndCBzYXkgaXQgd291bGQgcmVhbGx5IGFkZCBhbnl0
aGluZywganVzdCBteSBzZW5zZSBvZg0KPiBjb25zaXN0ZW5jeSBraWNraW5nIGluIDspDQo+IEFu
eWhvdywgbm90IHJlYWxseSBhbnl0aGluZyB0byBkbyB3aXRoIHRoaXMgZHJpdmVyIHJldmlldyBz
byBmZWVsDQo+IGZyZWUgdG8gaWdub3JlIQ0KPiANCj4gPiArCXN0cnVjdCBpaW9fZGV2ICppbmRp
b19kZXYgPSBwbGF0Zm9ybV9nZXRfZHJ2ZGF0YShwZGV2KTsNCj4gPiArCXN0cnVjdCB0ZW1wZXJh
dHVyZV9zdGF0ZSAqdGVtcGVyYXR1cmVfc3RhdGUgPQ0KPiA+IGlpb19wcml2KGluZGlvX2Rldik7
DQo+ID4gKw0KPiA+ICsJc2Vuc29yX2h1Yl9yZW1vdmVfY2FsbGJhY2soaHNkZXYsDQo+ID4gSElE
X1VTQUdFX1NFTlNPUl9URU1QRVJBVFVSRSk7DQo+ID4gKwloaWRfc2Vuc29yX3JlbW92ZV90cmln
Z2VyKCZ0ZW1wZXJhdHVyZV9zdGF0ZS0NCj4gPiA+Y29tbW9uX2F0dHJpYnV0ZXMpOw0KPiA+ICsN
Cj4gPiArCXJldHVybiAwOw0KPiA+ICt9DQo+ID4gKw0KPiA+ICtzdGF0aWMgY29uc3Qgc3RydWN0
IHBsYXRmb3JtX2RldmljZV9pZCBoaWRfdGVtcGVyYXR1cmVfaWRzW10gPSB7DQo+ID4gKwl7DQo+
ID4gKwkJLyogRm9ybWF0OiBISUQtU0VOU09SLXVzYWdlX2lkX2luX2hleF9sb3dlcmNhc2UgKi8N
Cj4gPiArCQkubmFtZSA9ICJISUQtU0VOU09SLTIwMDAzMyIsDQo+ID4gKwl9LA0KPiA+ICsJeyAv
KiBzZW50aW5lbCAqLyB9DQo+ID4gK307DQo+ID4gK01PRFVMRV9ERVZJQ0VfVEFCTEUocGxhdGZv
cm0sIGhpZF90ZW1wZXJhdHVyZV9pZHMpOw0KPiA+ICsNCj4gPiArc3RhdGljIHN0cnVjdCBwbGF0
Zm9ybV9kcml2ZXIgaGlkX3RlbXBlcmF0dXJlX3BsYXRmb3JtX2RyaXZlciA9IHsNCj4gPiArCS5p
ZF90YWJsZSA9IGhpZF90ZW1wZXJhdHVyZV9pZHMsDQo+ID4gKwkuZHJpdmVyID0gew0KPiA+ICsJ
CS5uYW1lCT0gS0JVSUxEX01PRE5BTUUsDQo+ID4gKwkJLnBtCT0gJmhpZF9zZW5zb3JfcG1fb3Bz
LA0KPiA+ICsJfSwNCj4gPiArCS5wcm9iZQkJPSBoaWRfdGVtcGVyYXR1cmVfcHJvYmUsDQo+ID4g
KwkucmVtb3ZlCQk9IGhpZF90ZW1wZXJhdHVyZV9yZW1vdmUsDQo+ID4gK307DQo+ID4gK21vZHVs
ZV9wbGF0Zm9ybV9kcml2ZXIoaGlkX3RlbXBlcmF0dXJlX3BsYXRmb3JtX2RyaXZlcik7DQo+ID4g
Kw0KPiA+ICtNT0RVTEVfREVTQ1JJUFRJT04oIkhJRCBFbnZpcm9ubWVudGFsIHRlbXBlcmF0dXJl
IHNlbnNvciIpOw0KPiA+ICtNT0RVTEVfQVVUSE9SKCJTb25nIEhvbmd5YW4gPGhvbmd5YW4uc29u
Z0BpbnRlbC5jb20+Iik7DQo+ID4gK01PRFVMRV9MSUNFTlNFKCJHUEwgdjIiKTsNCj4gPiBkaWZm
IC0tZ2l0IGEvaW5jbHVkZS9saW51eC9oaWQtc2Vuc29yLWlkcy5oIGIvaW5jbHVkZS9saW51eC9o
aWQtDQo+ID4gc2Vuc29yLWlkcy5oDQo+ID4gaW5kZXggYmFiODM3NS4uZWNlYzk3OCAxMDA2NDQN
Cj4gPiAtLS0gYS9pbmNsdWRlL2xpbnV4L2hpZC1zZW5zb3ItaWRzLmgNCj4gPiArKysgYi9pbmNs
dWRlL2xpbnV4L2hpZC1zZW5zb3ItaWRzLmgNCj4gPiBAQCAtNDUsNiArNDUsMTAgQEANCj4gPiDC
oCNkZWZpbmUNCj4gPiBISURfVVNBR0VfU0VOU09SX0RBVEFfQVRNT1NQSEVSSUNfUFJFU1NVUkXC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgMHgyMDA0MzANCj4gPiDCoCNkZWZpbmUNCj4gPiBI
SURfVVNBR0VfU0VOU09SX0FUTU9TUEhFUklDX1BSRVNTVVJFwqDCoMKgwqDCoMKgwqDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqAweDIwMDQzMQ0KPiA+IMKgDQo+ID4gKy8qIFRlbXByZXR1cmUgKDIw
MDAzMykgKi8NCj4gPiArI2RlZmluZQlISURfVVNBR0VfU0VOU09SX1RFTVBFUkFUVVJFCQkJDQo+
ID4gCTB4MjAwMDMzDQo+ID4gKyNkZWZpbmUJSElEX1VTQUdFX1NFTlNPUl9EQVRBX0VOVklST05N
RU5UQUxfVEVNUEVSQVRVUkUJDQo+ID4gCTB4MjAwNDM0DQo+ID4gKw0KPiA+IMKgLyogR3lybyAz
RDogKDIwMDA3NikgKi8NCj4gPiDCoCNkZWZpbmUgSElEX1VTQUdFX1NFTlNPUl9HWVJPXzNECQkJ
CTB4DQo+ID4gMjAwMDc2DQo+ID4gwqAjZGVmaW5lIEhJRF9VU0FHRV9TRU5TT1JfREFUQV9BTkdM
X1ZFTE9DSVRZCQkJDQo+ID4gMHgyMDA0NTYNCj4gPiANCj4gDQo+IA==

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-02-21 10:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-13  9:49 [PATCH v3] iio: hid: Add temperature sensor support Song Hongyan
     [not found] ` <1486979375-44684-1-git-send-email-hongyan.song-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2017-02-19 11:48   ` Jonathan Cameron
2017-02-19 11:48     ` Jonathan Cameron
2017-02-21 10:29     ` Pandruvada, Srinivas
2017-02-21 10:29       ` Pandruvada, Srinivas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.