Patchwork [4/4,v4] rtc: add rtc-driver for HID sensors of type time

login
register
mail settings
Submitter Alexander Holler
Date Dec. 12, 2012, 11:11 a.m.
Message ID <1355310680-2466-1-git-send-email-holler@ahsoftware.de>
Download mbox | patch
Permalink /patch/205483/
State New
Headers show

Comments

Alexander Holler - Dec. 12, 2012, 11:11 a.m.
This driver makes the time from HID sensors (hubs) which are offering
such available like any other RTC does.

Currently the time can only be read. Setting the time must be done
through sending a report, which currently isn't supported by
hid-sensor-hub. (I've planned to submit patches.)

It is necessary that all values like year, month etc, are send as
8bit values (1 byte each) and all of them in 1 report. Also the
spec HUTRR39b doesn't define the range of the year field, we
tread it as 0 - 99 because that's what most RTCs I know about are
offering.

Signed-off-by: Alexander Holler <holler@ahsoftware.de>
---
 drivers/rtc/Kconfig               |   16 ++
 drivers/rtc/Makefile              |    1 +
 drivers/rtc/rtc-hid-sensor-time.c |  283 +++++++++++++++++++++++++++++++++++++
 3 files changed, 300 insertions(+), 0 deletions(-)
 create mode 100644 drivers/rtc/rtc-hid-sensor-time.c
Alexander Holler - Dec. 14, 2012, 1:08 p.m.
Am 14.12.2012 10:42, schrieb Lars-Peter Clausen:
> On 12/12/2012 12:11 PM, Alexander Holler wrote:
>> This driver makes the time from HID sensors (hubs) which are offering
>> such available like any other RTC does.
>>
>> Currently the time can only be read. Setting the time must be done
>> through sending a report, which currently isn't supported by
>> hid-sensor-hub. (I've planned to submit patches.)
>>
>> It is necessary that all values like year, month etc, are send as
>> 8bit values (1 byte each) and all of them in 1 report. Also the
>> spec HUTRR39b doesn't define the range of the year field, we
>> tread it as 0 - 99 because that's what most RTCs I know about are
>> offering.
>>
>> Signed-off-by: Alexander Holler <holler@ahsoftware.de>
>
> Hi,
>
> sorry for the delay. There is still the __devinit in front of
> hid_time_remove left.
>
> And another thing I've overlooked before:
> wait_for_completion_interruptible_timeout can either return a positive
> number when the completion was completed, 0 in case of an timeout, or a
> negative error code in case it was interrupted. You need to handle all
> three. E.g. something like this.
>
> ret = wait_for_completion_interruptible_timeout(...)
> if (ret == 0)
> 	return -EIO;
> if (ret < 0)
> 	return ret
>

Hmpf, the only working approach to use some in kernel functions really 
is to the read source yourself and don't trust anything else. :/

And that ping-ping is stressing my patience, I think I will write a rfc 
to introduce (at least allow) maintainer-patches.

Will make a v5.

Regards,

Alexander
Alexander Holler - Dec. 14, 2012, 2:15 p.m.
Am 14.12.2012 14:08, schrieb Alexander Holler:
> Am 14.12.2012 10:42, schrieb Lars-Peter Clausen:

>> And another thing I've overlooked before:
>> wait_for_completion_interruptible_timeout can either return a positive
>> number when the completion was completed, 0 in case of an timeout, or a
>> negative error code in case it was interrupted. You need to handle all
>> three. E.g. something like this.
>>
>> ret = wait_for_completion_interruptible_timeout(...)
>> if (ret == 0)
>>     return -EIO;
>> if (ret < 0)
>>     return ret
>>
>
> Hmpf, the only working approach to use some in kernel functions really
> is to the read source yourself and don't trust anything else. :/

Anyway, my approach doesn't work as it introduces a race condition:


/* get a report with all values through requesting one value */
sensor_hub_input_attr_get_raw_value(...)

/* race if this task goes to slepp and the values were
received before it could call the below wait...

/* wait for all values (event) */
if (!wait_for_completion_interruptible_timeout(...))


I'll have to look for a mechanism how to avoid that. So v5 might need 
some time.


Regards,

Alexander
Alexander Holler - Dec. 14, 2012, 2:29 p.m.
Am 14.12.2012 15:15, schrieb Alexander Holler:
> Am 14.12.2012 14:08, schrieb Alexander Holler:
>> Am 14.12.2012 10:42, schrieb Lars-Peter Clausen:
>
>>> And another thing I've overlooked before:
>>> wait_for_completion_interruptible_timeout can either return a positive
>>> number when the completion was completed, 0 in case of an timeout, or a
>>> negative error code in case it was interrupted. You need to handle all
>>> three. E.g. something like this.
>>>
>>> ret = wait_for_completion_interruptible_timeout(...)
>>> if (ret == 0)
>>>     return -EIO;
>>> if (ret < 0)
>>>     return ret
>>>
>>
>> Hmpf, the only working approach to use some in kernel functions really
>> is to the read source yourself and don't trust anything else. :/
>
> Anyway, my approach doesn't work as it introduces a race condition:
>
>
> /* get a report with all values through requesting one value */
> sensor_hub_input_attr_get_raw_value(...)
>
> /* race if this task goes to slepp and the values were
> received before it could call the below wait...
>
> /* wait for all values (event) */
> if (!wait_for_completion_interruptible_timeout(...))
>
>
> I'll have to look for a mechanism how to avoid that. So v5 might need
> some time.

Sorry for the noise. That INIT_COMPLETION() before the sensor...() does 
exactly that. So it's enough if I handle the different return situations 
of wait_for...().

I will just use if(wait...()<=0) return -EIO.

Regards,

Alexander
Lars-Peter Clausen - Dec. 14, 2012, 2:34 p.m.
On 12/14/2012 03:29 PM, Alexander Holler wrote:
> Am 14.12.2012 15:15, schrieb Alexander Holler:
>> Am 14.12.2012 14:08, schrieb Alexander Holler:
>>> Am 14.12.2012 10:42, schrieb Lars-Peter Clausen:
>>
>>>> And another thing I've overlooked before:
>>>> wait_for_completion_interruptible_timeout can either return a positive
>>>> number when the completion was completed, 0 in case of an timeout, or a
>>>> negative error code in case it was interrupted. You need to handle all
>>>> three. E.g. something like this.
>>>>
>>>> ret = wait_for_completion_interruptible_timeout(...)
>>>> if (ret == 0)
>>>>     return -EIO;
>>>> if (ret < 0)
>>>>     return ret
>>>>
>>>
>>> Hmpf, the only working approach to use some in kernel functions really
>>> is to the read source yourself and don't trust anything else. :/
>>
>> Anyway, my approach doesn't work as it introduces a race condition:
>>
>>
>> /* get a report with all values through requesting one value */
>> sensor_hub_input_attr_get_raw_value(...)
>>
>> /* race if this task goes to slepp and the values were
>> received before it could call the below wait...
>>
>> /* wait for all values (event) */
>> if (!wait_for_completion_interruptible_timeout(...))
>>
>>
>> I'll have to look for a mechanism how to avoid that. So v5 might need
>> some time.
> 
> Sorry for the noise. That INIT_COMPLETION() before the sensor...() does
> exactly that. So it's enough if I handle the different return situations of
> wait_for...().
> 
> I will just use if(wait...()<=0) return -EIO.
> 

No, that's wrong. You should really return the error code returned by
wait_for_completion_interruptible_timeout(). This will make sure that
userspace restarts the syscall if necessary.

- Lars
Alexander Holler - Dec. 14, 2012, 3:24 p.m.
Am 14.12.2012 15:34, schrieb Lars-Peter Clausen:
> On 12/14/2012 03:29 PM, Alexander Holler wrote:
>> Am 14.12.2012 15:15, schrieb Alexander Holler:
>>> Am 14.12.2012 14:08, schrieb Alexander Holler:
>>>> Am 14.12.2012 10:42, schrieb Lars-Peter Clausen:
>>>
>>>>> And another thing I've overlooked before:
>>>>> wait_for_completion_interruptible_timeout can either return a positive
>>>>> number when the completion was completed, 0 in case of an timeout, or a
>>>>> negative error code in case it was interrupted. You need to handle all
>>>>> three. E.g. something like this.
>>>>>
>>>>> ret = wait_for_completion_interruptible_timeout(...)
>>>>> if (ret == 0)
>>>>>     return -EIO;
>>>>> if (ret < 0)
>>>>>     return ret
>>>>>
>>>>
>>>> Hmpf, the only working approach to use some in kernel functions really
>>>> is to the read source yourself and don't trust anything else. :/
>>>
>>> Anyway, my approach doesn't work as it introduces a race condition:
>>>
>>>
>>> /* get a report with all values through requesting one value */
>>> sensor_hub_input_attr_get_raw_value(...)
>>>
>>> /* race if this task goes to slepp and the values were
>>> received before it could call the below wait...
>>>
>>> /* wait for all values (event) */
>>> if (!wait_for_completion_interruptible_timeout(...))
>>>
>>>
>>> I'll have to look for a mechanism how to avoid that. So v5 might need
>>> some time.
>>
>> Sorry for the noise. That INIT_COMPLETION() before the sensor...() does
>> exactly that. So it's enough if I handle the different return situations of
>> wait_for...().
>>
>> I will just use if(wait...()<=0) return -EIO.
>>
> 
> No, that's wrong. You should really return the error code returned by
> wait_for_completion_interruptible_timeout(). This will make sure that
> userspace restarts the syscall if necessary.

Sorry for my ignorance, but which reasons for interruption do exist
which doesn't kill the userspace too? The error number -ESYSRESTART
doesn't offer a hint.

Alexander
Lars-Peter Clausen - Dec. 14, 2012, 4:33 p.m.
On 12/14/2012 04:24 PM, Alexander Holler wrote:
> Am 14.12.2012 15:34, schrieb Lars-Peter Clausen:
>> On 12/14/2012 03:29 PM, Alexander Holler wrote:
>>> Am 14.12.2012 15:15, schrieb Alexander Holler:
>>>> Am 14.12.2012 14:08, schrieb Alexander Holler:
>>>>> Am 14.12.2012 10:42, schrieb Lars-Peter Clausen:
>>>>
>>>>>> And another thing I've overlooked before:
>>>>>> wait_for_completion_interruptible_timeout can either return a positive
>>>>>> number when the completion was completed, 0 in case of an timeout, or a
>>>>>> negative error code in case it was interrupted. You need to handle all
>>>>>> three. E.g. something like this.
>>>>>>
>>>>>> ret = wait_for_completion_interruptible_timeout(...)
>>>>>> if (ret == 0)
>>>>>>     return -EIO;
>>>>>> if (ret < 0)
>>>>>>     return ret
>>>>>>
>>>>>
>>>>> Hmpf, the only working approach to use some in kernel functions really
>>>>> is to the read source yourself and don't trust anything else. :/
>>>>
>>>> Anyway, my approach doesn't work as it introduces a race condition:
>>>>
>>>>
>>>> /* get a report with all values through requesting one value */
>>>> sensor_hub_input_attr_get_raw_value(...)
>>>>
>>>> /* race if this task goes to slepp and the values were
>>>> received before it could call the below wait...
>>>>
>>>> /* wait for all values (event) */
>>>> if (!wait_for_completion_interruptible_timeout(...))
>>>>
>>>>
>>>> I'll have to look for a mechanism how to avoid that. So v5 might need
>>>> some time.
>>>
>>> Sorry for the noise. That INIT_COMPLETION() before the sensor...() does
>>> exactly that. So it's enough if I handle the different return situations of
>>> wait_for...().
>>>
>>> I will just use if(wait...()<=0) return -EIO.
>>>
>>
>> No, that's wrong. You should really return the error code returned by
>> wait_for_completion_interruptible_timeout(). This will make sure that
>> userspace restarts the syscall if necessary.
> 
> Sorry for my ignorance, but which reasons for interruption do exist
> which doesn't kill the userspace too? The error number -ESYSRESTART
> doesn't offer a hint.


Well I'm not an expert on this either, but as far as I know any signal the
process is listening on can cause an interruption. Most signals won't kill
the process though. More on the whole restart stuff:
http://lwn.net/Articles/528935/

- Lars
Alexander Holler - Dec. 14, 2012, 9:24 p.m.
Am 14.12.2012 17:33, schrieb Lars-Peter Clausen:
> On 12/14/2012 04:24 PM, Alexander Holler wrote:
>> Am 14.12.2012 15:34, schrieb Lars-Peter Clausen:
>>> On 12/14/2012 03:29 PM, Alexander Holler wrote:
>>>> Am 14.12.2012 15:15, schrieb Alexander Holler:
>>>>> Am 14.12.2012 14:08, schrieb Alexander Holler:
>>>>>> Am 14.12.2012 10:42, schrieb Lars-Peter Clausen:
>>>>>
>>>>>>> And another thing I've overlooked before:
>>>>>>> wait_for_completion_interruptible_timeout can either return a positive
>>>>>>> number when the completion was completed, 0 in case of an timeout, or a
>>>>>>> negative error code in case it was interrupted. You need to handle all
>>>>>>> three. E.g. something like this.
>>>>>>>
>>>>>>> ret = wait_for_completion_interruptible_timeout(...)
>>>>>>> if (ret == 0)
>>>>>>>      return -EIO;
>>>>>>> if (ret < 0)
>>>>>>>      return ret
>>>>>>>
>>>>>>
>>>>>> Hmpf, the only working approach to use some in kernel functions really
>>>>>> is to the read source yourself and don't trust anything else. :/
>>>>>
>>>>> Anyway, my approach doesn't work as it introduces a race condition:
>>>>>
>>>>>
>>>>> /* get a report with all values through requesting one value */
>>>>> sensor_hub_input_attr_get_raw_value(...)
>>>>>
>>>>> /* race if this task goes to slepp and the values were
>>>>> received before it could call the below wait...
>>>>>
>>>>> /* wait for all values (event) */
>>>>> if (!wait_for_completion_interruptible_timeout(...))
>>>>>
>>>>>
>>>>> I'll have to look for a mechanism how to avoid that. So v5 might need
>>>>> some time.
>>>>
>>>> Sorry for the noise. That INIT_COMPLETION() before the sensor...() does
>>>> exactly that. So it's enough if I handle the different return situations of
>>>> wait_for...().
>>>>
>>>> I will just use if(wait...()<=0) return -EIO.
>>>>
>>>
>>> No, that's wrong. You should really return the error code returned by
>>> wait_for_completion_interruptible_timeout(). This will make sure that
>>> userspace restarts the syscall if necessary.
>>
>> Sorry for my ignorance, but which reasons for interruption do exist
>> which doesn't kill the userspace too? The error number -ESYSRESTART
>> doesn't offer a hint.
>
>
> Well I'm not an expert on this either, but as far as I know any signal the
> process is listening on can cause an interruption. Most signals won't kill
> the process though. More on the whole restart stuff:
> http://lwn.net/Articles/528935/

I've come to the conclusion that using 
wait_for_completion_interruptible_timeout() isn't what should be used 
here and will switch to wait_for_completion_killable_timeout(). Here are 
my reasons:

1. I don't think any caller of rtc_ops.read_time will be prepared to 
handle -ESYSRESTART.

2. Someone wants the time. Beeing interruptible seems to mean the 
process might be interrupted by signal processing with an unknown long 
duration which could decrease the accuracy even below the desired one 
second (which doesn't look unlikely, signal processing in userspace is 
unpredictable and (mis)used for all kind of stuff).

3. I've primarily used it to be friendly in means of killable, but 
didn't know that ..._killable() does exist. ;)

Btw., here is a imho good and especially short introduction about the 
task states along with some pointers to more comprehensive resources:
http://www.ibm.com/developerworks/linux/library/l-task-killable/

v5 of that patch will follow. Hopefully the last one necessary.

Regards,

Alexander

Patch

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 19c03ab..e0d29b5 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1144,4 +1144,20 @@  config RTC_DRV_SNVS
 	   This driver can also be built as a module, if so, the module
 	   will be called "rtc-snvs".
 
+comment "HID Sensor RTC drivers"
+
+config RTC_DRV_HID_SENSOR_TIME
+	tristate "HID Sensor Time"
+	depends on USB_HID
+	select IIO
+	select HID_SENSOR_HUB
+	select HID_SENSOR_IIO_COMMON
+	help
+	  Say yes here to build support for the HID Sensors of type Time.
+	  This drivers makes such sensors available as RTCs.
+
+	  If this driver is compiled as a module, it will be named
+	  rtc-hid-sensor-time.
+
+
 endif # RTC_CLASS
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 56297f0..9d1658a 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -51,6 +51,7 @@  obj-$(CONFIG_RTC_DRV_EM3027)	+= rtc-em3027.o
 obj-$(CONFIG_RTC_DRV_EP93XX)	+= rtc-ep93xx.o
 obj-$(CONFIG_RTC_DRV_FM3130)	+= rtc-fm3130.o
 obj-$(CONFIG_RTC_DRV_GENERIC)	+= rtc-generic.o
+obj-$(CONFIG_RTC_DRV_HID_SENSOR_TIME) += rtc-hid-sensor-time.o
 obj-$(CONFIG_RTC_DRV_IMXDI)	+= rtc-imxdi.o
 obj-$(CONFIG_RTC_DRV_ISL1208)	+= rtc-isl1208.o
 obj-$(CONFIG_RTC_DRV_ISL12022)	+= rtc-isl12022.o
diff --git a/drivers/rtc/rtc-hid-sensor-time.c b/drivers/rtc/rtc-hid-sensor-time.c
new file mode 100644
index 0000000..41bc011
--- /dev/null
+++ b/drivers/rtc/rtc-hid-sensor-time.c
@@ -0,0 +1,283 @@ 
+/*
+ * HID Sensor Time Driver
+ * Copyright (c) 2012, Alexander Holler.
+ *
+ * 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; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/hid-sensor-hub.h>
+#include <linux/iio/iio.h>
+#include <linux/rtc.h>
+
+/* Format: HID-SENSOR-usage_id_in_hex */
+/* Usage ID from spec for Time: 0x2000A0 */
+#define DRIVER_NAME "HID-SENSOR-2000a0" /* must be lowercase */
+
+enum hid_time_channel {
+	CHANNEL_SCAN_INDEX_YEAR,
+	CHANNEL_SCAN_INDEX_MONTH,
+	CHANNEL_SCAN_INDEX_DAY,
+	CHANNEL_SCAN_INDEX_HOUR,
+	CHANNEL_SCAN_INDEX_MINUTE,
+	CHANNEL_SCAN_INDEX_SECOND,
+	TIME_RTC_CHANNEL_MAX,
+};
+
+struct hid_time_state {
+	struct hid_sensor_hub_callbacks callbacks;
+	struct hid_sensor_iio_common common_attributes;
+	struct hid_sensor_hub_attribute_info info[TIME_RTC_CHANNEL_MAX];
+	struct rtc_time last_time;
+	spinlock_t lock_last_time;
+	struct completion comp_last_time;
+	struct rtc_time time_buf;
+	struct rtc_device *rtc;
+};
+
+static const u32 hid_time_addresses[TIME_RTC_CHANNEL_MAX] = {
+	HID_USAGE_SENSOR_TIME_YEAR,
+	HID_USAGE_SENSOR_TIME_MONTH,
+	HID_USAGE_SENSOR_TIME_DAY,
+	HID_USAGE_SENSOR_TIME_HOUR,
+	HID_USAGE_SENSOR_TIME_MINUTE,
+	HID_USAGE_SENSOR_TIME_SECOND,
+};
+
+/* Channel names for verbose error messages */
+static const char * const hid_time_channel_names[TIME_RTC_CHANNEL_MAX] = {
+	"year", "month", "day", "hour", "minute", "second",
+};
+
+/* Callback handler to send event after all samples are received and captured */
+static int hid_time_proc_event(struct hid_sensor_hub_device *hsdev,
+				unsigned usage_id, void *priv)
+{
+	unsigned long flags;
+	struct hid_time_state *time_state = platform_get_drvdata(priv);
+
+	spin_lock_irqsave(&time_state->lock_last_time, flags);
+	time_state->last_time = time_state->time_buf;
+	spin_unlock_irqrestore(&time_state->lock_last_time, flags);
+	complete(&time_state->comp_last_time);
+	return 0;
+}
+
+static int hid_time_capture_sample(struct hid_sensor_hub_device *hsdev,
+				unsigned usage_id, size_t raw_len,
+				char *raw_data, void *priv)
+{
+	struct hid_time_state *time_state = platform_get_drvdata(priv);
+	struct rtc_time *time_buf = &time_state->time_buf;
+
+	switch (usage_id) {
+	case HID_USAGE_SENSOR_TIME_YEAR:
+		time_buf->tm_year = *(u8 *)raw_data;
+		if (time_buf->tm_year < 70)
+			/* assume we are in 1970...2069 */
+			time_buf->tm_year += 100;
+		break;
+	case HID_USAGE_SENSOR_TIME_MONTH:
+		/* sensor sending the month as 1-12, we need 0-11 */
+		time_buf->tm_mon = *(u8 *)raw_data-1;
+		break;
+	case HID_USAGE_SENSOR_TIME_DAY:
+		time_buf->tm_mday = *(u8 *)raw_data;
+		break;
+	case HID_USAGE_SENSOR_TIME_HOUR:
+		time_buf->tm_hour = *(u8 *)raw_data;
+		break;
+	case HID_USAGE_SENSOR_TIME_MINUTE:
+		time_buf->tm_min = *(u8 *)raw_data;
+		break;
+	case HID_USAGE_SENSOR_TIME_SECOND:
+		time_buf->tm_sec = *(u8 *)raw_data;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+/* small helper, haven't found any other way */
+static const char *attrib_name(u32 attrib_id)
+{
+	static const char unknown[] = "unknown";
+	unsigned i;
+
+	for (i = 0; i < TIME_RTC_CHANNEL_MAX; ++i) {
+		if (hid_time_addresses[i] == attrib_id)
+			return hid_time_channel_names[i];
+	}
+	return unknown; /* should never happen */
+}
+
+static int hid_time_parse_report(struct platform_device *pdev,
+				struct hid_sensor_hub_device *hsdev,
+				unsigned usage_id,
+				struct hid_time_state *time_state)
+{
+	int report_id, i;
+
+	for (i = 0; i < TIME_RTC_CHANNEL_MAX; ++i)
+		if (sensor_hub_input_get_attribute_info(hsdev,
+				HID_INPUT_REPORT, usage_id,
+				hid_time_addresses[i],
+				&time_state->info[i]) < 0)
+			return -EINVAL;
+	/* Check the (needed) attributes for sanity */
+	report_id = time_state->info[0].report_id;
+	if (report_id < 0) {
+		dev_err(&pdev->dev, "bad report ID!\n");
+		return -EINVAL;
+	}
+	for (i = 0; i < TIME_RTC_CHANNEL_MAX; ++i) {
+		if (time_state->info[i].report_id != report_id) {
+			dev_err(&pdev->dev,
+				"not all needed attributes inside the same report!\n");
+			return -EINVAL;
+		}
+		if (time_state->info[i].size != 1) {
+			dev_err(&pdev->dev,
+				"attribute '%s' not 8 bits wide!\n",
+				attrib_name(time_state->info[i].attrib_id));
+			return -EINVAL;
+		}
+		if (time_state->info[i].units !=
+				HID_USAGE_SENSOR_UNITS_NOT_SPECIFIED &&
+				/* allow attribute seconds with unit seconds */
+				!(time_state->info[i].attrib_id ==
+				HID_USAGE_SENSOR_TIME_SECOND &&
+				time_state->info[i].units ==
+				HID_USAGE_SENSOR_UNITS_SECOND)) {
+			dev_err(&pdev->dev,
+				"attribute '%s' hasn't a unit of type 'none'!\n",
+				attrib_name(time_state->info[i].attrib_id));
+			return -EINVAL;
+		}
+		if (time_state->info[i].unit_expo) {
+			dev_err(&pdev->dev,
+				"attribute '%s' hasn't a unit exponent of 1!\n",
+				attrib_name(time_state->info[i].attrib_id));
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int hid_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	unsigned long flags;
+	struct hid_time_state *time_state =
+		platform_get_drvdata(to_platform_device(dev));
+
+	INIT_COMPLETION(time_state->comp_last_time);
+	/* get a report with all values through requesting one value */
+	sensor_hub_input_attr_get_raw_value(time_state->common_attributes.hsdev,
+			HID_USAGE_SENSOR_TIME, hid_time_addresses[0],
+			time_state->info[0].report_id);
+	/* wait for all values (event) */
+	if (!wait_for_completion_interruptible_timeout(
+			&time_state->comp_last_time, HZ*6))
+		return -EIO;
+	spin_lock_irqsave(&time_state->lock_last_time, flags);
+	*tm = time_state->last_time;
+	spin_unlock_irqrestore(&time_state->lock_last_time, flags);
+
+	return 0;
+}
+
+static const struct rtc_class_ops rtc_ops = {
+	.read_time = hid_rtc_read_time,
+};
+
+static int hid_time_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
+	struct hid_time_state *time_state = devm_kzalloc(&pdev->dev,
+		sizeof(struct hid_time_state), GFP_KERNEL);
+
+	if (time_state == NULL)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, time_state);
+
+	init_completion(&time_state->comp_last_time);
+	time_state->common_attributes.hsdev = hsdev;
+	time_state->common_attributes.pdev = pdev;
+
+	ret = hid_sensor_parse_common_attributes(hsdev,
+				HID_USAGE_SENSOR_TIME,
+				&time_state->common_attributes);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to setup common attributes!\n");
+		return ret;
+	}
+
+	ret = hid_time_parse_report(pdev, hsdev, HID_USAGE_SENSOR_TIME,
+					time_state);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to setup attributes!\n");
+		return ret;
+	}
+
+	time_state->callbacks.send_event = hid_time_proc_event;
+	time_state->callbacks.capture_sample = hid_time_capture_sample;
+	time_state->callbacks.pdev = pdev;
+	ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_TIME,
+					&time_state->callbacks);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "register callback failed!\n");
+		return ret;
+	}
+
+	time_state->rtc = rtc_device_register("hid-sensor-time",
+				&pdev->dev, &rtc_ops, THIS_MODULE);
+
+	if (IS_ERR(time_state->rtc)) {
+		dev_err(&pdev->dev, "rtc device register failed!\n");
+		return PTR_ERR(time_state->rtc);
+	}
+
+	return ret;
+}
+
+static int __devinit hid_time_remove(struct platform_device *pdev)
+{
+	struct hid_sensor_hub_device *hsdev = pdev->dev.platform_data;
+	struct hid_time_state *time_state = platform_get_drvdata(pdev);
+
+	rtc_device_unregister(time_state->rtc);
+	sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_TIME);
+
+	return 0;
+}
+
+static struct platform_driver hid_time_platform_driver = {
+	.driver = {
+		.name	= DRIVER_NAME,
+		.owner	= THIS_MODULE,
+	},
+	.probe		= hid_time_probe,
+	.remove		= hid_time_remove,
+};
+module_platform_driver(hid_time_platform_driver);
+
+MODULE_DESCRIPTION("HID Sensor Time");
+MODULE_AUTHOR("Alexander Holler <holler@ahsoftware.de>");
+MODULE_LICENSE("GPL");