[PATCH v3 1/4] mfd: cros_ec: Add helper for event notifier.

[PATCH v3 1/4] mfd: cros_ec: Add helper for event notifier.

[Enric Balletbo i Serra]

From: Gwendal Grignou <gwendal@chromium.org>

Add cros_ec_get_event() entry point to retrieve event within functions
called by the notifier.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes since v2:
 - none
Changes since v1:
 - Acked by Lee Jones

 drivers/platform/chrome/cros_ec_proto.c | 20 ++++++++++++++++++++
 include/linux/mfd/cros_ec.h             | 10 ++++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index ed5dee7..7428c2b 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -494,3 +494,23 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
 		return get_keyboard_state_event(ec_dev);
 }
 EXPORT_SYMBOL(cros_ec_get_next_event);
+
+u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev)
+{
+	u32 host_event;
+
+	BUG_ON(!ec_dev->mkbp_event_supported);
+
+	if (ec_dev->event_data.event_type != EC_MKBP_EVENT_HOST_EVENT)
+		return 0;
+
+	if (ec_dev->event_size != sizeof(host_event)) {
+		dev_warn(ec_dev->dev, "Invalid host event size\n");
+		return 0;
+	}
+
+	host_event = get_unaligned_le32(&ec_dev->event_data.data.host_event);
+
+	return host_event;
+}
+EXPORT_SYMBOL(cros_ec_get_host_event);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index b3d04de..be2c4eb 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -299,6 +299,16 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev);
  */
 int cros_ec_get_next_event(struct cros_ec_device *ec_dev);
 
+/**
+ * cros_ec_get_host_event - Return a mask of event set by the EC.
+ *
+ * When MKBP is supported, when the EC raises an interrupt,
+ * We collect the events raised and call the functions in the ec notifier.
+ *
+ * This function is a helper to know which events are raised.
+ */
+u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
+
 /* sysfs stuff */
 extern struct attribute_group cros_ec_attr_group;
 extern struct attribute_group cros_ec_lightbar_attr_group;
-- 
2.9.3

[rtc-linux] [PATCH v3 1/4] mfd: cros_ec: Add helper for event notifier.

[Enric Balletbo i Serra]

From: Gwendal Grignou <gwendal@chromium.org>

Add cros_ec_get_event() entry point to retrieve event within functions
called by the notifier.

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes since v2:
 - none
Changes since v1:
 - Acked by Lee Jones

 drivers/platform/chrome/cros_ec_proto.c | 20 ++++++++++++++++++++
 include/linux/mfd/cros_ec.h             | 10 ++++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index ed5dee7..7428c2b 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -494,3 +494,23 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
 		return get_keyboard_state_event(ec_dev);
 }
 EXPORT_SYMBOL(cros_ec_get_next_event);
+
+u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev)
+{
+	u32 host_event;
+
+	BUG_ON(!ec_dev->mkbp_event_supported);
+
+	if (ec_dev->event_data.event_type != EC_MKBP_EVENT_HOST_EVENT)
+		return 0;
+
+	if (ec_dev->event_size != sizeof(host_event)) {
+		dev_warn(ec_dev->dev, "Invalid host event size\n");
+		return 0;
+	}
+
+	host_event = get_unaligned_le32(&ec_dev->event_data.data.host_event);
+
+	return host_event;
+}
+EXPORT_SYMBOL(cros_ec_get_host_event);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index b3d04de..be2c4eb 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -299,6 +299,16 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev);
  */
 int cros_ec_get_next_event(struct cros_ec_device *ec_dev);
 
+/**
+ * cros_ec_get_host_event - Return a mask of event set by the EC.
+ *
+ * When MKBP is supported, when the EC raises an interrupt,
+ * We collect the events raised and call the functions in the ec notifier.
+ *
+ * This function is a helper to know which events are raised.
+ */
+u32 cros_ec_get_host_event(struct cros_ec_device *ec_dev);
+
 /* sysfs stuff */
 extern struct attribute_group cros_ec_attr_group;
 extern struct attribute_group cros_ec_lightbar_attr_group;
-- 
2.9.3

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 2/4] mfd: cros_ec: Introduce RTC commands and events definitions.

[Enric Balletbo i Serra]

From: Stephen Barber <smbarber@chromium.org>

The EC can function as a simple RT, this patch adds the RTC related
definitions needed by the rtc-cros-ec driver.

Signed-off-by: Stephen Barber <smbarber@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes since v2:
 - none
Changes since v1:
 - Acked by Lee Jones

 include/linux/mfd/cros_ec_commands.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 80d401d..73f7a62 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -286,6 +286,9 @@ enum host_event_code {
 	/* Hang detect logic detected a hang and warm rebooted the AP */
 	EC_HOST_EVENT_HANG_REBOOT = 21,
 
+	/* EC RTC event occurred */
+	EC_HOST_EVENT_RTC = 26,
+
 	/*
 	 * The high bit of the event mask is not used as a host event code.  If
 	 * it reads back as set, then the entire event mask should be
@@ -790,6 +793,8 @@ enum ec_feature_code {
 	EC_FEATURE_USB_MUX = 23,
 	/* Motion Sensor code has an internal software FIFO */
 	EC_FEATURE_MOTION_SENSE_FIFO = 24,
+	/* EC has RTC feature that can be controlled by host commands */
+	EC_FEATURE_RTC = 27,
 };
 
 #define EC_FEATURE_MASK_0(event_code) (1UL << (event_code % 32))
@@ -1682,6 +1687,9 @@ struct ec_response_rtc {
 #define EC_CMD_RTC_SET_VALUE 0x46
 #define EC_CMD_RTC_SET_ALARM 0x47
 
+/* Pass as param to SET_ALARM to clear the current alarm */
+#define EC_RTC_ALARM_CLEAR 0
+
 /*****************************************************************************/
 /* Port80 log access */
 
-- 
2.9.3

[rtc-linux] [PATCH v3 2/4] mfd: cros_ec: Introduce RTC commands and events definitions.

[Enric Balletbo i Serra]

From: Stephen Barber <smbarber@chromium.org>

The EC can function as a simple RT, this patch adds the RTC related
definitions needed by the rtc-cros-ec driver.

Signed-off-by: Stephen Barber <smbarber@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
Changes since v2:
 - none
Changes since v1:
 - Acked by Lee Jones

 include/linux/mfd/cros_ec_commands.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 80d401d..73f7a62 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -286,6 +286,9 @@ enum host_event_code {
 	/* Hang detect logic detected a hang and warm rebooted the AP */
 	EC_HOST_EVENT_HANG_REBOOT = 21,
 
+	/* EC RTC event occurred */
+	EC_HOST_EVENT_RTC = 26,
+
 	/*
 	 * The high bit of the event mask is not used as a host event code.  If
 	 * it reads back as set, then the entire event mask should be
@@ -790,6 +793,8 @@ enum ec_feature_code {
 	EC_FEATURE_USB_MUX = 23,
 	/* Motion Sensor code has an internal software FIFO */
 	EC_FEATURE_MOTION_SENSE_FIFO = 24,
+	/* EC has RTC feature that can be controlled by host commands */
+	EC_FEATURE_RTC = 27,
 };
 
 #define EC_FEATURE_MASK_0(event_code) (1UL << (event_code % 32))
@@ -1682,6 +1687,9 @@ struct ec_response_rtc {
 #define EC_CMD_RTC_SET_VALUE 0x46
 #define EC_CMD_RTC_SET_ALARM 0x47
 
+/* Pass as param to SET_ALARM to clear the current alarm */
+#define EC_RTC_ALARM_CLEAR 0
+
 /*****************************************************************************/
 /* Port80 log access */
 
-- 
2.9.3

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 3/4] rtc: cros-ec: add cros-ec-rtc driver.

[Enric Balletbo i Serra]

From: Stephen Barber <smbarber@chromium.org>

On platforms with a Chrome OS EC, the EC can function as a simple RTC.
Add a basic driver with this functionality.

Signed-off-by: Stephen Barber <smbarber@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
Changes since v2:
 - Acked by Alexandre Belloni
Changes since v1:
 - Moved RTC_DRV_CROS_EC after "Platform RTC drivers" (Alexandre Belloni)
 - Remove check for invalid date/time as is not useful (Alexandre Belloni)

 drivers/rtc/Kconfig       |  10 ++
 drivers/rtc/Makefile      |   1 +
 drivers/rtc/rtc-cros-ec.c | 413 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 424 insertions(+)
 create mode 100644 drivers/rtc/rtc-cros-ec.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index c93c5a8..59ab3d6 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1204,6 +1204,16 @@ config RTC_DRV_ZYNQMP
 	  If you say yes here you get support for the RTC controller found on
 	  Xilinx Zynq Ultrascale+ MPSoC.
 
+config RTC_DRV_CROS_EC
+	tristate "Chrome OS EC RTC driver"
+	depends on MFD_CROS_EC
+	help
+	  If you say yes here you will get support for the
+	  Chrome OS Embedded Controller's RTC.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-cros-ec.
+
 comment "on-CPU RTC drivers"
 
 config RTC_DRV_ASM9260
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index f13ab1c..dd753e6 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_RTC_DRV_BQ32K)	+= rtc-bq32k.o
 obj-$(CONFIG_RTC_DRV_BQ4802)	+= rtc-bq4802.o
 obj-$(CONFIG_RTC_DRV_CMOS)	+= rtc-cmos.o
 obj-$(CONFIG_RTC_DRV_COH901331)	+= rtc-coh901331.o
+obj-$(CONFIG_RTC_DRV_CROS_EC)	+= rtc-cros-ec.o
 obj-$(CONFIG_RTC_DRV_DA9052)	+= rtc-da9052.o
 obj-$(CONFIG_RTC_DRV_DA9055)	+= rtc-da9055.o
 obj-$(CONFIG_RTC_DRV_DA9063)	+= rtc-da9063.o
diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c
new file mode 100644
index 0000000..f0ea689
--- /dev/null
+++ b/drivers/rtc/rtc-cros-ec.c
@@ -0,0 +1,413 @@
+/*
+ * RTC driver for Chrome OS Embedded Controller
+ *
+ * Copyright (c) 2017, Google, Inc
+ *
+ * Author: Stephen Barber <smbarber@chromium.org>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+
+#define DRV_NAME	"cros-ec-rtc"
+
+/**
+ * struct cros_ec_rtc - Driver data for EC RTC
+ *
+ * @cros_ec: Pointer to EC device
+ * @rtc: Pointer to RTC device
+ * @notifier: Notifier info for responding to EC events
+ * @saved_alarm: Alarm to restore when interrupts are reenabled
+ */
+struct cros_ec_rtc {
+	struct cros_ec_device *cros_ec;
+	struct rtc_device *rtc;
+	struct notifier_block notifier;
+	u32 saved_alarm;
+};
+
+static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command,
+			   u32 *response)
+{
+	int ret;
+	struct {
+		struct cros_ec_command msg;
+		struct ec_response_rtc data;
+	} __packed msg;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.msg.command = command;
+	msg.msg.insize = sizeof(msg.data);
+
+	ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+	if (ret < 0) {
+		dev_err(cros_ec->dev,
+			"error getting %s from EC: %d\n",
+			command == EC_CMD_RTC_GET_VALUE ? "time" : "alarm",
+			ret);
+		return ret;
+	}
+
+	*response = msg.data.time;
+
+	return 0;
+}
+
+static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command,
+			   u32 param)
+{
+	int ret = 0;
+	struct {
+		struct cros_ec_command msg;
+		struct ec_response_rtc data;
+	} __packed msg;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.msg.command = command;
+	msg.msg.outsize = sizeof(msg.data);
+	msg.data.time = param;
+
+	ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+	if (ret < 0) {
+		dev_err(cros_ec->dev, "error setting %s on EC: %d\n",
+			command == EC_CMD_RTC_SET_VALUE ? "time" : "alarm",
+			ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+/* Read the current time from the EC. */
+static int cros_ec_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	u32 time;
+
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &time);
+	if (ret) {
+		dev_err(dev, "error getting time: %d\n", ret);
+		return ret;
+	}
+
+	rtc_time64_to_tm(time, tm);
+
+	return 0;
+}
+
+/* Set the current EC time. */
+static int cros_ec_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	time64_t time;
+
+	time = rtc_tm_to_time64(tm);
+	if (time < 0 || time > U32_MAX)
+		return -EINVAL;
+
+	ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_VALUE, (u32)time);
+	if (ret < 0) {
+		dev_err(dev, "error setting time: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+/* Read alarm time from RTC. */
+static int cros_ec_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	u32 current_time, alarm_offset;
+
+	/*
+	 * The EC host command for getting the alarm is relative (i.e. 5
+	 * seconds from now) whereas rtc_wkalrm is absolute. Get the current
+	 * RTC time first so we can calculate the relative time.
+	 */
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &current_time);
+	if (ret < 0) {
+		dev_err(dev, "error getting time: %d\n", ret);
+		return ret;
+	}
+
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_ALARM, &alarm_offset);
+	if (ret < 0) {
+		dev_err(dev, "error getting alarm: %d\n", ret);
+		return ret;
+	}
+
+	rtc_time64_to_tm(current_time + alarm_offset, &alrm->time);
+
+	return 0;
+}
+
+/* Set the EC's RTC alarm. */
+static int cros_ec_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	time64_t alarm_time;
+	u32 current_time, alarm_offset;
+
+	/*
+	 * The EC host command for setting the alarm is relative
+	 * (i.e. 5 seconds from now) whereas rtc_wkalrm is absolute.
+	 * Get the current RTC time first so we can calculate the
+	 * relative time.
+	 */
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &current_time);
+	if (ret < 0) {
+		dev_err(dev, "error getting time: %d\n", ret);
+		return ret;
+	}
+
+	alarm_time = rtc_tm_to_time64(&alrm->time);
+
+	if (alarm_time < 0 || alarm_time > U32_MAX)
+		return -EINVAL;
+
+	if (!alrm->enabled) {
+		/*
+		 * If the alarm is being disabled, send an alarm
+		 * clear command.
+		 */
+		alarm_offset = EC_RTC_ALARM_CLEAR;
+		cros_ec_rtc->saved_alarm = (u32)alarm_time;
+	} else {
+		/* Don't set an alarm in the past. */
+		if ((u32)alarm_time < current_time)
+			alarm_offset = EC_RTC_ALARM_CLEAR;
+		else
+			alarm_offset = (u32)alarm_time - current_time;
+	}
+
+	ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM, alarm_offset);
+	if (ret < 0) {
+		dev_err(dev, "error setting alarm: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int cros_ec_rtc_alarm_irq_enable(struct device *dev,
+					unsigned int enabled)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	u32 current_time, alarm_offset, alarm_value;
+
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &current_time);
+	if (ret < 0) {
+		dev_err(dev, "error getting time: %d\n", ret);
+		return ret;
+	}
+
+	if (enabled) {
+		/* Restore saved alarm if it's still in the future. */
+		if (cros_ec_rtc->saved_alarm < current_time)
+			alarm_offset = EC_RTC_ALARM_CLEAR;
+		else
+			alarm_offset = cros_ec_rtc->saved_alarm - current_time;
+
+		ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM,
+				      alarm_offset);
+		if (ret < 0) {
+			dev_err(dev, "error restoring alarm: %d\n", ret);
+			return ret;
+		}
+	} else {
+		/* Disable alarm, saving the old alarm value. */
+		ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_ALARM,
+				      &alarm_offset);
+		if (ret < 0) {
+			dev_err(dev, "error saving alarm: %d\n", ret);
+			return ret;
+		}
+
+		alarm_value = current_time + alarm_offset;
+
+		/*
+		 * If the current EC alarm is already past, we don't want
+		 * to set an alarm when we go through the alarm irq enable
+		 * path.
+		 */
+		if (alarm_value < current_time)
+			cros_ec_rtc->saved_alarm = EC_RTC_ALARM_CLEAR;
+		else
+			cros_ec_rtc->saved_alarm = alarm_value;
+
+		alarm_offset = EC_RTC_ALARM_CLEAR;
+		ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM,
+				      alarm_offset);
+		if (ret < 0) {
+			dev_err(dev, "error disabling alarm: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int cros_ec_rtc_event(struct notifier_block *nb,
+			     unsigned long queued_during_suspend,
+			     void *_notify)
+{
+	struct cros_ec_rtc *cros_ec_rtc;
+	struct rtc_device *rtc;
+	struct cros_ec_device *cros_ec;
+	u32 host_event;
+
+	cros_ec_rtc = container_of(nb, struct cros_ec_rtc, notifier);
+	rtc = cros_ec_rtc->rtc;
+	cros_ec = cros_ec_rtc->cros_ec;
+
+	host_event = cros_ec_get_host_event(cros_ec);
+	if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_RTC)) {
+		rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
+		return NOTIFY_OK;
+	} else {
+		return NOTIFY_DONE;
+	}
+}
+
+static const struct rtc_class_ops cros_ec_rtc_ops = {
+	.read_time = cros_ec_rtc_read_time,
+	.set_time = cros_ec_rtc_set_time,
+	.read_alarm = cros_ec_rtc_read_alarm,
+	.set_alarm = cros_ec_rtc_set_alarm,
+	.alarm_irq_enable = cros_ec_rtc_alarm_irq_enable,
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int cros_ec_rtc_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(&pdev->dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(cros_ec_rtc->cros_ec->irq);
+
+	return 0;
+}
+
+static int cros_ec_rtc_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(&pdev->dev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(cros_ec_rtc->cros_ec->irq);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(cros_ec_rtc_pm_ops, cros_ec_rtc_suspend,
+			 cros_ec_rtc_resume);
+
+static int cros_ec_rtc_probe(struct platform_device *pdev)
+{
+	struct cros_ec_dev *ec_dev = dev_get_drvdata(pdev->dev.parent);
+	struct cros_ec_device *cros_ec = ec_dev->ec_dev;
+	struct cros_ec_rtc *cros_ec_rtc;
+	struct rtc_time tm;
+	int ret;
+
+	cros_ec_rtc = devm_kzalloc(&pdev->dev, sizeof(*cros_ec_rtc),
+				   GFP_KERNEL);
+	if (!cros_ec_rtc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, cros_ec_rtc);
+	cros_ec_rtc->cros_ec = cros_ec;
+
+	/* Get initial time */
+	ret = cros_ec_rtc_read_time(&pdev->dev, &tm);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to read RTC time\n");
+		return ret;
+	}
+
+	ret = device_init_wakeup(&pdev->dev, 1);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize wakeup\n");
+		return ret;
+	}
+
+	cros_ec_rtc->rtc = devm_rtc_device_register(&pdev->dev, DRV_NAME,
+						    &cros_ec_rtc_ops,
+						    THIS_MODULE);
+	if (IS_ERR(cros_ec_rtc->rtc)) {
+		ret = PTR_ERR(cros_ec_rtc->rtc);
+		dev_err(&pdev->dev, "failed to register rtc device\n");
+		return ret;
+	}
+
+	/* Get RTC events from the EC. */
+	cros_ec_rtc->notifier.notifier_call = cros_ec_rtc_event;
+	ret = blocking_notifier_chain_register(&cros_ec->event_notifier,
+					       &cros_ec_rtc->notifier);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register notifier\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int cros_ec_rtc_remove(struct platform_device *pdev)
+{
+	struct cros_ec_rtc *cros_ec_rtc = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = blocking_notifier_chain_unregister(
+				&cros_ec_rtc->cros_ec->event_notifier,
+				&cros_ec_rtc->notifier);
+	if (ret) {
+		dev_err(dev, "failed to unregister notifier\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver cros_ec_rtc_driver = {
+	.probe = cros_ec_rtc_probe,
+	.remove = cros_ec_rtc_remove,
+	.driver = {
+		.name = DRV_NAME,
+		.pm = &cros_ec_rtc_pm_ops,
+	},
+};
+
+module_platform_driver(cros_ec_rtc_driver);
+
+MODULE_DESCRIPTION("RTC driver for Chrome OS ECs");
+MODULE_AUTHOR("Stephen Barber <smbarber@chromium.org>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRV_NAME);
-- 
2.9.3

[rtc-linux] [PATCH v3 3/4] rtc: cros-ec: add cros-ec-rtc driver.

[Enric Balletbo i Serra]

From: Stephen Barber <smbarber@chromium.org>

On platforms with a Chrome OS EC, the EC can function as a simple RTC.
Add a basic driver with this functionality.

Signed-off-by: Stephen Barber <smbarber@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---
Changes since v2:
 - Acked by Alexandre Belloni
Changes since v1:
 - Moved RTC_DRV_CROS_EC after "Platform RTC drivers" (Alexandre Belloni)
 - Remove check for invalid date/time as is not useful (Alexandre Belloni)

 drivers/rtc/Kconfig       |  10 ++
 drivers/rtc/Makefile      |   1 +
 drivers/rtc/rtc-cros-ec.c | 413 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 424 insertions(+)
 create mode 100644 drivers/rtc/rtc-cros-ec.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index c93c5a8..59ab3d6 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1204,6 +1204,16 @@ config RTC_DRV_ZYNQMP
 	  If you say yes here you get support for the RTC controller found on
 	  Xilinx Zynq Ultrascale+ MPSoC.
 
+config RTC_DRV_CROS_EC
+	tristate "Chrome OS EC RTC driver"
+	depends on MFD_CROS_EC
+	help
+	  If you say yes here you will get support for the
+	  Chrome OS Embedded Controller's RTC.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-cros-ec.
+
 comment "on-CPU RTC drivers"
 
 config RTC_DRV_ASM9260
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index f13ab1c..dd753e6 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_RTC_DRV_BQ32K)	+= rtc-bq32k.o
 obj-$(CONFIG_RTC_DRV_BQ4802)	+= rtc-bq4802.o
 obj-$(CONFIG_RTC_DRV_CMOS)	+= rtc-cmos.o
 obj-$(CONFIG_RTC_DRV_COH901331)	+= rtc-coh901331.o
+obj-$(CONFIG_RTC_DRV_CROS_EC)	+= rtc-cros-ec.o
 obj-$(CONFIG_RTC_DRV_DA9052)	+= rtc-da9052.o
 obj-$(CONFIG_RTC_DRV_DA9055)	+= rtc-da9055.o
 obj-$(CONFIG_RTC_DRV_DA9063)	+= rtc-da9063.o
diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c
new file mode 100644
index 0000000..f0ea689
--- /dev/null
+++ b/drivers/rtc/rtc-cros-ec.c
@@ -0,0 +1,413 @@
+/*
+ * RTC driver for Chrome OS Embedded Controller
+ *
+ * Copyright (c) 2017, Google, Inc
+ *
+ * Author: Stephen Barber <smbarber@chromium.org>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+
+#define DRV_NAME	"cros-ec-rtc"
+
+/**
+ * struct cros_ec_rtc - Driver data for EC RTC
+ *
+ * @cros_ec: Pointer to EC device
+ * @rtc: Pointer to RTC device
+ * @notifier: Notifier info for responding to EC events
+ * @saved_alarm: Alarm to restore when interrupts are reenabled
+ */
+struct cros_ec_rtc {
+	struct cros_ec_device *cros_ec;
+	struct rtc_device *rtc;
+	struct notifier_block notifier;
+	u32 saved_alarm;
+};
+
+static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command,
+			   u32 *response)
+{
+	int ret;
+	struct {
+		struct cros_ec_command msg;
+		struct ec_response_rtc data;
+	} __packed msg;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.msg.command = command;
+	msg.msg.insize = sizeof(msg.data);
+
+	ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+	if (ret < 0) {
+		dev_err(cros_ec->dev,
+			"error getting %s from EC: %d\n",
+			command == EC_CMD_RTC_GET_VALUE ? "time" : "alarm",
+			ret);
+		return ret;
+	}
+
+	*response = msg.data.time;
+
+	return 0;
+}
+
+static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command,
+			   u32 param)
+{
+	int ret = 0;
+	struct {
+		struct cros_ec_command msg;
+		struct ec_response_rtc data;
+	} __packed msg;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.msg.command = command;
+	msg.msg.outsize = sizeof(msg.data);
+	msg.data.time = param;
+
+	ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+	if (ret < 0) {
+		dev_err(cros_ec->dev, "error setting %s on EC: %d\n",
+			command == EC_CMD_RTC_SET_VALUE ? "time" : "alarm",
+			ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+/* Read the current time from the EC. */
+static int cros_ec_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	u32 time;
+
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &time);
+	if (ret) {
+		dev_err(dev, "error getting time: %d\n", ret);
+		return ret;
+	}
+
+	rtc_time64_to_tm(time, tm);
+
+	return 0;
+}
+
+/* Set the current EC time. */
+static int cros_ec_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	time64_t time;
+
+	time = rtc_tm_to_time64(tm);
+	if (time < 0 || time > U32_MAX)
+		return -EINVAL;
+
+	ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_VALUE, (u32)time);
+	if (ret < 0) {
+		dev_err(dev, "error setting time: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+/* Read alarm time from RTC. */
+static int cros_ec_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	u32 current_time, alarm_offset;
+
+	/*
+	 * The EC host command for getting the alarm is relative (i.e. 5
+	 * seconds from now) whereas rtc_wkalrm is absolute. Get the current
+	 * RTC time first so we can calculate the relative time.
+	 */
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &current_time);
+	if (ret < 0) {
+		dev_err(dev, "error getting time: %d\n", ret);
+		return ret;
+	}
+
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_ALARM, &alarm_offset);
+	if (ret < 0) {
+		dev_err(dev, "error getting alarm: %d\n", ret);
+		return ret;
+	}
+
+	rtc_time64_to_tm(current_time + alarm_offset, &alrm->time);
+
+	return 0;
+}
+
+/* Set the EC's RTC alarm. */
+static int cros_ec_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	time64_t alarm_time;
+	u32 current_time, alarm_offset;
+
+	/*
+	 * The EC host command for setting the alarm is relative
+	 * (i.e. 5 seconds from now) whereas rtc_wkalrm is absolute.
+	 * Get the current RTC time first so we can calculate the
+	 * relative time.
+	 */
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &current_time);
+	if (ret < 0) {
+		dev_err(dev, "error getting time: %d\n", ret);
+		return ret;
+	}
+
+	alarm_time = rtc_tm_to_time64(&alrm->time);
+
+	if (alarm_time < 0 || alarm_time > U32_MAX)
+		return -EINVAL;
+
+	if (!alrm->enabled) {
+		/*
+		 * If the alarm is being disabled, send an alarm
+		 * clear command.
+		 */
+		alarm_offset = EC_RTC_ALARM_CLEAR;
+		cros_ec_rtc->saved_alarm = (u32)alarm_time;
+	} else {
+		/* Don't set an alarm in the past. */
+		if ((u32)alarm_time < current_time)
+			alarm_offset = EC_RTC_ALARM_CLEAR;
+		else
+			alarm_offset = (u32)alarm_time - current_time;
+	}
+
+	ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM, alarm_offset);
+	if (ret < 0) {
+		dev_err(dev, "error setting alarm: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int cros_ec_rtc_alarm_irq_enable(struct device *dev,
+					unsigned int enabled)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	u32 current_time, alarm_offset, alarm_value;
+
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &current_time);
+	if (ret < 0) {
+		dev_err(dev, "error getting time: %d\n", ret);
+		return ret;
+	}
+
+	if (enabled) {
+		/* Restore saved alarm if it's still in the future. */
+		if (cros_ec_rtc->saved_alarm < current_time)
+			alarm_offset = EC_RTC_ALARM_CLEAR;
+		else
+			alarm_offset = cros_ec_rtc->saved_alarm - current_time;
+
+		ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM,
+				      alarm_offset);
+		if (ret < 0) {
+			dev_err(dev, "error restoring alarm: %d\n", ret);
+			return ret;
+		}
+	} else {
+		/* Disable alarm, saving the old alarm value. */
+		ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_ALARM,
+				      &alarm_offset);
+		if (ret < 0) {
+			dev_err(dev, "error saving alarm: %d\n", ret);
+			return ret;
+		}
+
+		alarm_value = current_time + alarm_offset;
+
+		/*
+		 * If the current EC alarm is already past, we don't want
+		 * to set an alarm when we go through the alarm irq enable
+		 * path.
+		 */
+		if (alarm_value < current_time)
+			cros_ec_rtc->saved_alarm = EC_RTC_ALARM_CLEAR;
+		else
+			cros_ec_rtc->saved_alarm = alarm_value;
+
+		alarm_offset = EC_RTC_ALARM_CLEAR;
+		ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM,
+				      alarm_offset);
+		if (ret < 0) {
+			dev_err(dev, "error disabling alarm: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int cros_ec_rtc_event(struct notifier_block *nb,
+			     unsigned long queued_during_suspend,
+			     void *_notify)
+{
+	struct cros_ec_rtc *cros_ec_rtc;
+	struct rtc_device *rtc;
+	struct cros_ec_device *cros_ec;
+	u32 host_event;
+
+	cros_ec_rtc = container_of(nb, struct cros_ec_rtc, notifier);
+	rtc = cros_ec_rtc->rtc;
+	cros_ec = cros_ec_rtc->cros_ec;
+
+	host_event = cros_ec_get_host_event(cros_ec);
+	if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_RTC)) {
+		rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
+		return NOTIFY_OK;
+	} else {
+		return NOTIFY_DONE;
+	}
+}
+
+static const struct rtc_class_ops cros_ec_rtc_ops = {
+	.read_time = cros_ec_rtc_read_time,
+	.set_time = cros_ec_rtc_set_time,
+	.read_alarm = cros_ec_rtc_read_alarm,
+	.set_alarm = cros_ec_rtc_set_alarm,
+	.alarm_irq_enable = cros_ec_rtc_alarm_irq_enable,
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int cros_ec_rtc_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(&pdev->dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(cros_ec_rtc->cros_ec->irq);
+
+	return 0;
+}
+
+static int cros_ec_rtc_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(&pdev->dev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(cros_ec_rtc->cros_ec->irq);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(cros_ec_rtc_pm_ops, cros_ec_rtc_suspend,
+			 cros_ec_rtc_resume);
+
+static int cros_ec_rtc_probe(struct platform_device *pdev)
+{
+	struct cros_ec_dev *ec_dev = dev_get_drvdata(pdev->dev.parent);
+	struct cros_ec_device *cros_ec = ec_dev->ec_dev;
+	struct cros_ec_rtc *cros_ec_rtc;
+	struct rtc_time tm;
+	int ret;
+
+	cros_ec_rtc = devm_kzalloc(&pdev->dev, sizeof(*cros_ec_rtc),
+				   GFP_KERNEL);
+	if (!cros_ec_rtc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, cros_ec_rtc);
+	cros_ec_rtc->cros_ec = cros_ec;
+
+	/* Get initial time */
+	ret = cros_ec_rtc_read_time(&pdev->dev, &tm);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to read RTC time\n");
+		return ret;
+	}
+
+	ret = device_init_wakeup(&pdev->dev, 1);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize wakeup\n");
+		return ret;
+	}
+
+	cros_ec_rtc->rtc = devm_rtc_device_register(&pdev->dev, DRV_NAME,
+						    &cros_ec_rtc_ops,
+						    THIS_MODULE);
+	if (IS_ERR(cros_ec_rtc->rtc)) {
+		ret = PTR_ERR(cros_ec_rtc->rtc);
+		dev_err(&pdev->dev, "failed to register rtc device\n");
+		return ret;
+	}
+
+	/* Get RTC events from the EC. */
+	cros_ec_rtc->notifier.notifier_call = cros_ec_rtc_event;
+	ret = blocking_notifier_chain_register(&cros_ec->event_notifier,
+					       &cros_ec_rtc->notifier);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register notifier\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int cros_ec_rtc_remove(struct platform_device *pdev)
+{
+	struct cros_ec_rtc *cros_ec_rtc = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = blocking_notifier_chain_unregister(
+				&cros_ec_rtc->cros_ec->event_notifier,
+				&cros_ec_rtc->notifier);
+	if (ret) {
+		dev_err(dev, "failed to unregister notifier\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver cros_ec_rtc_driver = {
+	.probe = cros_ec_rtc_probe,
+	.remove = cros_ec_rtc_remove,
+	.driver = {
+		.name = DRV_NAME,
+		.pm = &cros_ec_rtc_pm_ops,
+	},
+};
+
+module_platform_driver(cros_ec_rtc_driver);
+
+MODULE_DESCRIPTION("RTC driver for Chrome OS ECs");
+MODULE_AUTHOR("Stephen Barber <smbarber@chromium.org>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRV_NAME);
-- 
2.9.3

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Enric Balletbo i Serra]

From: Stephen Barber <smbarber@chromium.org>

If the EC supports RTC host commands, expose an RTC device.

Signed-off-by: Stephen Barber <smbarber@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Benson Leung <bleung@chromium.org>
---
Changes since v2:
 - Acked by Benson Leung
Changes since v1:
 - none

 drivers/platform/chrome/cros_ec_dev.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index 47268ec..ebe029d 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -383,6 +383,24 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
 	kfree(msg);
 }
 
+static const struct mfd_cell cros_ec_rtc_devs[] = {
+	{
+		.name = "cros-ec-rtc",
+		.id   = -1,
+	},
+};
+
+static void cros_ec_rtc_register(struct cros_ec_dev *ec)
+{
+	int ret;
+
+	ret = mfd_add_devices(ec->dev, 0, cros_ec_rtc_devs,
+			      ARRAY_SIZE(cros_ec_rtc_devs),
+			      NULL, 0, NULL);
+	if (ret)
+		dev_err(ec->dev, "failed to add cros-ec-rtc device: %d\n", ret);
+}
+
 static int ec_device_probe(struct platform_device *pdev)
 {
 	int retval = -ENOMEM;
@@ -441,6 +459,10 @@ static int ec_device_probe(struct platform_device *pdev)
 	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
 		cros_ec_sensors_register(ec);
 
+	/* check whether this EC instance has RTC host command support */
+	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
+		cros_ec_rtc_register(ec);
+
 	return 0;
 
 dev_reg_failed:
-- 
2.9.3

[rtc-linux] [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Enric Balletbo i Serra]

From: Stephen Barber <smbarber@chromium.org>

If the EC supports RTC host commands, expose an RTC device.

Signed-off-by: Stephen Barber <smbarber@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Benson Leung <bleung@chromium.org>
---
Changes since v2:
 - Acked by Benson Leung
Changes since v1:
 - none

 drivers/platform/chrome/cros_ec_dev.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index 47268ec..ebe029d 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -383,6 +383,24 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
 	kfree(msg);
 }
 
+static const struct mfd_cell cros_ec_rtc_devs[] = {
+	{
+		.name = "cros-ec-rtc",
+		.id   = -1,
+	},
+};
+
+static void cros_ec_rtc_register(struct cros_ec_dev *ec)
+{
+	int ret;
+
+	ret = mfd_add_devices(ec->dev, 0, cros_ec_rtc_devs,
+			      ARRAY_SIZE(cros_ec_rtc_devs),
+			      NULL, 0, NULL);
+	if (ret)
+		dev_err(ec->dev, "failed to add cros-ec-rtc device: %d\n", ret);
+}
+
 static int ec_device_probe(struct platform_device *pdev)
 {
 	int retval = -ENOMEM;
@@ -441,6 +459,10 @@ static int ec_device_probe(struct platform_device *pdev)
 	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
 		cros_ec_sensors_register(ec);
 
+	/* check whether this EC instance has RTC host command support */
+	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
+		cros_ec_rtc_register(ec);
+
 	return 0;
 
 dev_reg_failed:
-- 
2.9.3

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Lee Jones]

On Tue, 14 Feb 2017, Enric Balletbo i Serra wrote:

> From: Stephen Barber <smbarber@chromium.org>
> 
> If the EC supports RTC host commands, expose an RTC device.
> 
> Signed-off-by: Stephen Barber <smbarber@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Acked-by: Benson Leung <bleung@chromium.org>
> ---
> Changes since v2:
>  - Acked by Benson Leung
> Changes since v1:
>  - none
> 
>  drivers/platform/chrome/cros_ec_dev.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
> index 47268ec..ebe029d 100644
> --- a/drivers/platform/chrome/cros_ec_dev.c
> +++ b/drivers/platform/chrome/cros_ec_dev.c
> @@ -383,6 +383,24 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
>  	kfree(msg);
>  }
>  
> +static const struct mfd_cell cros_ec_rtc_devs[] = {
> +	{
> +		.name = "cros-ec-rtc",
> +		.id   = -1,
> +	},
> +};
> +
> +static void cros_ec_rtc_register(struct cros_ec_dev *ec)
> +{
> +	int ret;
> +
> +	ret = mfd_add_devices(ec->dev, 0, cros_ec_rtc_devs,
> +			      ARRAY_SIZE(cros_ec_rtc_devs),
> +			      NULL, 0, NULL);
> +	if (ret)
> +		dev_err(ec->dev, "failed to add cros-ec-rtc device: %d\n", ret);
> +}

Holey poop!  Why are you using the MFD API outside of MFD?

Why can't you register this from the MFD driver?

>  static int ec_device_probe(struct platform_device *pdev)
>  {
>  	int retval = -ENOMEM;
> @@ -441,6 +459,10 @@ static int ec_device_probe(struct platform_device *pdev)
>  	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
>  		cros_ec_sensors_register(ec);
>  
> +	/* check whether this EC instance has RTC host command support */
> +	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
> +		cros_ec_rtc_register(ec);
> +
>  	return 0;
>  
>  dev_reg_failed:

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[rtc-linux] Re: [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Lee Jones]

On Tue, 14 Feb 2017, Enric Balletbo i Serra wrote:

> From: Stephen Barber <smbarber@chromium.org>
>=20
> If the EC supports RTC host commands, expose an RTC device.
>=20
> Signed-off-by: Stephen Barber <smbarber@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Acked-by: Benson Leung <bleung@chromium.org>
> ---
> Changes since v2:
>  - Acked by Benson Leung
> Changes since v1:
>  - none
>=20
>  drivers/platform/chrome/cros_ec_dev.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>=20
> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chr=
ome/cros_ec_dev.c
> index 47268ec..ebe029d 100644
> --- a/drivers/platform/chrome/cros_ec_dev.c
> +++ b/drivers/platform/chrome/cros_ec_dev.c
> @@ -383,6 +383,24 @@ static void cros_ec_sensors_register(struct cros_ec_=
dev *ec)
>  	kfree(msg);
>  }
> =20
> +static const struct mfd_cell cros_ec_rtc_devs[] =3D {
> +	{
> +		.name =3D "cros-ec-rtc",
> +		.id   =3D -1,
> +	},
> +};
> +
> +static void cros_ec_rtc_register(struct cros_ec_dev *ec)
> +{
> +	int ret;
> +
> +	ret =3D mfd_add_devices(ec->dev, 0, cros_ec_rtc_devs,
> +			      ARRAY_SIZE(cros_ec_rtc_devs),
> +			      NULL, 0, NULL);
> +	if (ret)
> +		dev_err(ec->dev, "failed to add cros-ec-rtc device: %d\n", ret);
> +}

Holey poop!  Why are you using the MFD API outside of MFD?

Why can't you register this from the MFD driver?

>  static int ec_device_probe(struct platform_device *pdev)
>  {
>  	int retval =3D -ENOMEM;
> @@ -441,6 +459,10 @@ static int ec_device_probe(struct platform_device *p=
dev)
>  	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
>  		cros_ec_sensors_register(ec);
> =20
> +	/* check whether this EC instance has RTC host command support */
> +	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
> +		cros_ec_rtc_register(ec);
> +
>  	return 0;
> =20
>  dev_reg_failed:

--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Enric Balletbo i Serra]

Hi Lee,

On 14/03/17 14:59, Lee Jones wrote:
> On Tue, 14 Feb 2017, Enric Balletbo i Serra wrote:
> 
>> From: Stephen Barber <smbarber@chromium.org>
>>
>> If the EC supports RTC host commands, expose an RTC device.
>>
>> Signed-off-by: Stephen Barber <smbarber@chromium.org>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> Acked-by: Benson Leung <bleung@chromium.org>
>> ---
>> Changes since v2:
>>  - Acked by Benson Leung
>> Changes since v1:
>>  - none
>>
>>  drivers/platform/chrome/cros_ec_dev.c | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
>> index 47268ec..ebe029d 100644
>> --- a/drivers/platform/chrome/cros_ec_dev.c
>> +++ b/drivers/platform/chrome/cros_ec_dev.c
>> @@ -383,6 +383,24 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
>>  	kfree(msg);
>>  }
>>  
>> +static const struct mfd_cell cros_ec_rtc_devs[] = {
>> +	{
>> +		.name = "cros-ec-rtc",
>> +		.id   = -1,
>> +	},
>> +};
>> +
>> +static void cros_ec_rtc_register(struct cros_ec_dev *ec)
>> +{
>> +	int ret;
>> +
>> +	ret = mfd_add_devices(ec->dev, 0, cros_ec_rtc_devs,
>> +			      ARRAY_SIZE(cros_ec_rtc_devs),
>> +			      NULL, 0, NULL);
>> +	if (ret)
>> +		dev_err(ec->dev, "failed to add cros-ec-rtc device: %d\n", ret);
>> +}
> 
> Holey poop!  Why are you using the MFD API outside of MFD?
> 
> Why can't you register this from the MFD driver?
> 

Actually the MFD doesn't know how to check if this feature is available or not,
instead is the platform driver cros_ec_dev who knows how to check this and if it
exists adds the rtc device.

if (cros_ec_check_features(ec, EC_FEATURE_RTC))
	cros_ec_rtc_register(ec); /* add the mfd device */

Same approach was used in the same file for the Sensors Hub (already upstream). See:

  drivers/platform/chrome/cros_ec_dev.c:462
  drivers/platform/chrome/cros_ec_dev.c:372

I didn't know that the MFD API was restricted outside MFD. In such case what I
need to do is let know the MFD driver about the cros_ec_check_features
(implemented in platform driver cros_ec_dev), this doesn't seems good to me but
I might be wrong. So please, let me know which option do you prefer and if it's
the case we will need to change I'll try to do it.

Note that I think that a similar use case is used in
drivers/iio/common/ssp_sensors/ssp_dev.c:535, where the iio driver registers the
sensors to the mfd.

Thanks,
  Enric

>>  static int ec_device_probe(struct platform_device *pdev)
>>  {
>>  	int retval = -ENOMEM;
>> @@ -441,6 +459,10 @@ static int ec_device_probe(struct platform_device *pdev)
>>  	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
>>  		cros_ec_sensors_register(ec);
>>  
>> +	/* check whether this EC instance has RTC host command support */
>> +	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
>> +		cros_ec_rtc_register(ec);
>> +
>>  	return 0;
>>  
>>  dev_reg_failed:
> 

[rtc-linux] Re: [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Enric Balletbo i Serra]

Hi Lee,

On 14/03/17 14:59, Lee Jones wrote:
> On Tue, 14 Feb 2017, Enric Balletbo i Serra wrote:
> 
>> From: Stephen Barber <smbarber@chromium.org>
>>
>> If the EC supports RTC host commands, expose an RTC device.
>>
>> Signed-off-by: Stephen Barber <smbarber@chromium.org>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> Acked-by: Benson Leung <bleung@chromium.org>
>> ---
>> Changes since v2:
>>  - Acked by Benson Leung
>> Changes since v1:
>>  - none
>>
>>  drivers/platform/chrome/cros_ec_dev.c | 22 ++++++++++++++++++++++
>>  1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
>> index 47268ec..ebe029d 100644
>> --- a/drivers/platform/chrome/cros_ec_dev.c
>> +++ b/drivers/platform/chrome/cros_ec_dev.c
>> @@ -383,6 +383,24 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
>>  	kfree(msg);
>>  }
>>  
>> +static const struct mfd_cell cros_ec_rtc_devs[] = {
>> +	{
>> +		.name = "cros-ec-rtc",
>> +		.id   = -1,
>> +	},
>> +};
>> +
>> +static void cros_ec_rtc_register(struct cros_ec_dev *ec)
>> +{
>> +	int ret;
>> +
>> +	ret = mfd_add_devices(ec->dev, 0, cros_ec_rtc_devs,
>> +			      ARRAY_SIZE(cros_ec_rtc_devs),
>> +			      NULL, 0, NULL);
>> +	if (ret)
>> +		dev_err(ec->dev, "failed to add cros-ec-rtc device: %d\n", ret);
>> +}
> 
> Holey poop!  Why are you using the MFD API outside of MFD?
> 
> Why can't you register this from the MFD driver?
> 

Actually the MFD doesn't know how to check if this feature is available or not,
instead is the platform driver cros_ec_dev who knows how to check this and if it
exists adds the rtc device.

if (cros_ec_check_features(ec, EC_FEATURE_RTC))
	cros_ec_rtc_register(ec); /* add the mfd device */

Same approach was used in the same file for the Sensors Hub (already upstream). See:

  drivers/platform/chrome/cros_ec_dev.c:462
  drivers/platform/chrome/cros_ec_dev.c:372

I didn't know that the MFD API was restricted outside MFD. In such case what I
need to do is let know the MFD driver about the cros_ec_check_features
(implemented in platform driver cros_ec_dev), this doesn't seems good to me but
I might be wrong. So please, let me know which option do you prefer and if it's
the case we will need to change I'll try to do it.

Note that I think that a similar use case is used in
drivers/iio/common/ssp_sensors/ssp_dev.c:535, where the iio driver registers the
sensors to the mfd.

Thanks,
  Enric

>>  static int ec_device_probe(struct platform_device *pdev)
>>  {
>>  	int retval = -ENOMEM;
>> @@ -441,6 +459,10 @@ static int ec_device_probe(struct platform_device *pdev)
>>  	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
>>  		cros_ec_sensors_register(ec);
>>  
>> +	/* check whether this EC instance has RTC host command support */
>> +	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
>> +		cros_ec_rtc_register(ec);
>> +
>>  	return 0;
>>  
>>  dev_reg_failed:
> 

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Lee Jones]

On Tue, 14 Mar 2017, Enric Balletbo i Serra wrote:
> On 14/03/17 14:59, Lee Jones wrote:
> > On Tue, 14 Feb 2017, Enric Balletbo i Serra wrote:
> > 
> >> From: Stephen Barber <smbarber@chromium.org>
> >>
> >> If the EC supports RTC host commands, expose an RTC device.
> >>
> >> Signed-off-by: Stephen Barber <smbarber@chromium.org>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >> Acked-by: Benson Leung <bleung@chromium.org>
> >> ---
> >> Changes since v2:
> >>  - Acked by Benson Leung
> >> Changes since v1:
> >>  - none
> >>
> >>  drivers/platform/chrome/cros_ec_dev.c | 22 ++++++++++++++++++++++
> >>  1 file changed, 22 insertions(+)
> >>
> >> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
> >> index 47268ec..ebe029d 100644
> >> --- a/drivers/platform/chrome/cros_ec_dev.c
> >> +++ b/drivers/platform/chrome/cros_ec_dev.c
> >> @@ -383,6 +383,24 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
> >>  	kfree(msg);
> >>  }
> >>  
> >> +static const struct mfd_cell cros_ec_rtc_devs[] = {
> >> +	{
> >> +		.name = "cros-ec-rtc",
> >> +		.id   = -1,
> >> +	},
> >> +};
> >> +
> >> +static void cros_ec_rtc_register(struct cros_ec_dev *ec)
> >> +{
> >> +	int ret;
> >> +
> >> +	ret = mfd_add_devices(ec->dev, 0, cros_ec_rtc_devs,
> >> +			      ARRAY_SIZE(cros_ec_rtc_devs),
> >> +			      NULL, 0, NULL);
> >> +	if (ret)
> >> +		dev_err(ec->dev, "failed to add cros-ec-rtc device: %d\n", ret);
> >> +}
> > 
> > Holey poop!  Why are you using the MFD API outside of MFD?
> > 
> > Why can't you register this from the MFD driver?
> > 
> 
> Actually the MFD doesn't know how to check if this feature is available or not,
> instead is the platform driver cros_ec_dev who knows how to check this and if it
> exists adds the rtc device.
> 
> if (cros_ec_check_features(ec, EC_FEATURE_RTC))
> 	cros_ec_rtc_register(ec); /* add the mfd device */
> 
> Same approach was used in the same file for the Sensors Hub (already upstream). See:
> 
>   drivers/platform/chrome/cros_ec_dev.c:462
>   drivers/platform/chrome/cros_ec_dev.c:372
> 
> I didn't know that the MFD API was restricted outside MFD. In such case what I
> need to do is let know the MFD driver about the cros_ec_check_features
> (implemented in platform driver cros_ec_dev), this doesn't seems good to me but
> I might be wrong. So please, let me know which option do you prefer and if it's
> the case we will need to change I'll try to do it.
> 
> Note that I think that a similar use case is used in
> drivers/iio/common/ssp_sensors/ssp_dev.c:535, where the iio driver registers the
> sensors to the mfd.

It would be advantageous to avoid a web of inter-subsystem calls to
register devices.  I think I could bear calls to mfd_add_* from
drivers/platform, as the two subsystems are fairly interchangeable,
and it does have the added benefit of saving duplication of the device
registering code.  Calling mfd_add_* from IIO seems plain wrong though.

A better solution however and one that we've utilised in the past is
to have the MFD drivers call into the platform (i.e. drivers/platform)
drivers to see if certain devices are available.  Is this possible in
your use-case?

NB: I've just had a look at the SSP IIO driver and I have a number of
problems with it.  You should not be using that as a good example of
why mfd_add_* should be used outside of MFD.

> >>  static int ec_device_probe(struct platform_device *pdev)
> >>  {
> >>  	int retval = -ENOMEM;
> >> @@ -441,6 +459,10 @@ static int ec_device_probe(struct platform_device *pdev)
> >>  	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
> >>  		cros_ec_sensors_register(ec);
> >>  
> >> +	/* check whether this EC instance has RTC host command support */
> >> +	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
> >> +		cros_ec_rtc_register(ec);
> >> +
> >>  	return 0;
> >>  
> >>  dev_reg_failed:
> > 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[rtc-linux] Re: [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Lee Jones]

On Tue, 14 Mar 2017, Enric Balletbo i Serra wrote:
> On 14/03/17 14:59, Lee Jones wrote:
> > On Tue, 14 Feb 2017, Enric Balletbo i Serra wrote:
> >=20
> >> From: Stephen Barber <smbarber@chromium.org>
> >>
> >> If the EC supports RTC host commands, expose an RTC device.
> >>
> >> Signed-off-by: Stephen Barber <smbarber@chromium.org>
> >> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >> Acked-by: Benson Leung <bleung@chromium.org>
> >> ---
> >> Changes since v2:
> >>  - Acked by Benson Leung
> >> Changes since v1:
> >>  - none
> >>
> >>  drivers/platform/chrome/cros_ec_dev.c | 22 ++++++++++++++++++++++
> >>  1 file changed, 22 insertions(+)
> >>
> >> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/=
chrome/cros_ec_dev.c
> >> index 47268ec..ebe029d 100644
> >> --- a/drivers/platform/chrome/cros_ec_dev.c
> >> +++ b/drivers/platform/chrome/cros_ec_dev.c
> >> @@ -383,6 +383,24 @@ static void cros_ec_sensors_register(struct cros_=
ec_dev *ec)
> >>  	kfree(msg);
> >>  }
> >> =20
> >> +static const struct mfd_cell cros_ec_rtc_devs[] =3D {
> >> +	{
> >> +		.name =3D "cros-ec-rtc",
> >> +		.id   =3D -1,
> >> +	},
> >> +};
> >> +
> >> +static void cros_ec_rtc_register(struct cros_ec_dev *ec)
> >> +{
> >> +	int ret;
> >> +
> >> +	ret =3D mfd_add_devices(ec->dev, 0, cros_ec_rtc_devs,
> >> +			      ARRAY_SIZE(cros_ec_rtc_devs),
> >> +			      NULL, 0, NULL);
> >> +	if (ret)
> >> +		dev_err(ec->dev, "failed to add cros-ec-rtc device: %d\n", ret);
> >> +}
> >=20
> > Holey poop!  Why are you using the MFD API outside of MFD?
> >=20
> > Why can't you register this from the MFD driver?
> >=20
>=20
> Actually the MFD doesn't know how to check if this feature is available o=
r not,
> instead is the platform driver cros_ec_dev who knows how to check this an=
d if it
> exists adds the rtc device.
>=20
> if (cros_ec_check_features(ec, EC_FEATURE_RTC))
> 	cros_ec_rtc_register(ec); /* add the mfd device */
>=20
> Same approach was used in the same file for the Sensors Hub (already upst=
ream). See:
>=20
>   drivers/platform/chrome/cros_ec_dev.c:462
>   drivers/platform/chrome/cros_ec_dev.c:372
>=20
> I didn't know that the MFD API was restricted outside MFD. In such case w=
hat I
> need to do is let know the MFD driver about the cros_ec_check_features
> (implemented in platform driver cros_ec_dev), this doesn't seems good to =
me but
> I might be wrong. So please, let me know which option do you prefer and i=
f it's
> the case we will need to change I'll try to do it.
>=20
> Note that I think that a similar use case is used in
> drivers/iio/common/ssp_sensors/ssp_dev.c:535, where the iio driver regist=
ers the
> sensors to the mfd.

It would be advantageous to avoid a web of inter-subsystem calls to
register devices.  I think I could bear calls to mfd_add_* from
drivers/platform, as the two subsystems are fairly interchangeable,
and it does have the added benefit of saving duplication of the device
registering code.  Calling mfd_add_* from IIO seems plain wrong though.

A better solution however and one that we've utilised in the past is
to have the MFD drivers call into the platform (i.e. drivers/platform)
drivers to see if certain devices are available.  Is this possible in
your use-case?

NB: I've just had a look at the SSP IIO driver and I have a number of
problems with it.  You should not be using that as a good example of
why mfd_add_* should be used outside of MFD.

> >>  static int ec_device_probe(struct platform_device *pdev)
> >>  {
> >>  	int retval =3D -ENOMEM;
> >> @@ -441,6 +459,10 @@ static int ec_device_probe(struct platform_device=
 *pdev)
> >>  	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
> >>  		cros_ec_sensors_register(ec);
> >> =20
> >> +	/* check whether this EC instance has RTC host command support */
> >> +	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
> >> +		cros_ec_rtc_register(ec);
> >> +
> >>  	return 0;
> >> =20
> >>  dev_reg_failed:
> >=20

--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Enric Balletbo i Serra]

Hi Lee,

On 15/03/17 11:24, Lee Jones wrote:
> On Tue, 14 Mar 2017, Enric Balletbo i Serra wrote:
>> On 14/03/17 14:59, Lee Jones wrote:
>>> On Tue, 14 Feb 2017, Enric Balletbo i Serra wrote:
>>>
>>>> From: Stephen Barber <smbarber@chromium.org>
>>>>
>>>> If the EC supports RTC host commands, expose an RTC device.
>>>>
>>>> Signed-off-by: Stephen Barber <smbarber@chromium.org>
>>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>> Acked-by: Benson Leung <bleung@chromium.org>
>>>> ---
>>>> Changes since v2:
>>>>  - Acked by Benson Leung
>>>> Changes since v1:
>>>>  - none
>>>>
>>>>  drivers/platform/chrome/cros_ec_dev.c | 22 ++++++++++++++++++++++
>>>>  1 file changed, 22 insertions(+)
>>>>
>>>> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
>>>> index 47268ec..ebe029d 100644
>>>> --- a/drivers/platform/chrome/cros_ec_dev.c
>>>> +++ b/drivers/platform/chrome/cros_ec_dev.c
>>>> @@ -383,6 +383,24 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
>>>>  	kfree(msg);
>>>>  }
>>>>  
>>>> +static const struct mfd_cell cros_ec_rtc_devs[] = {
>>>> +	{
>>>> +		.name = "cros-ec-rtc",
>>>> +		.id   = -1,
>>>> +	},
>>>> +};
>>>> +
>>>> +static void cros_ec_rtc_register(struct cros_ec_dev *ec)
>>>> +{
>>>> +	int ret;
>>>> +
>>>> +	ret = mfd_add_devices(ec->dev, 0, cros_ec_rtc_devs,
>>>> +			      ARRAY_SIZE(cros_ec_rtc_devs),
>>>> +			      NULL, 0, NULL);
>>>> +	if (ret)
>>>> +		dev_err(ec->dev, "failed to add cros-ec-rtc device: %d\n", ret);
>>>> +}
>>>
>>> Holey poop!  Why are you using the MFD API outside of MFD?
>>>
>>> Why can't you register this from the MFD driver?
>>>
>>
>> Actually the MFD doesn't know how to check if this feature is available or not,
>> instead is the platform driver cros_ec_dev who knows how to check this and if it
>> exists adds the rtc device.
>>
>> if (cros_ec_check_features(ec, EC_FEATURE_RTC))
>> 	cros_ec_rtc_register(ec); /* add the mfd device */
>>
>> Same approach was used in the same file for the Sensors Hub (already upstream). See:
>>
>>   drivers/platform/chrome/cros_ec_dev.c:462
>>   drivers/platform/chrome/cros_ec_dev.c:372
>>
>> I didn't know that the MFD API was restricted outside MFD. In such case what I
>> need to do is let know the MFD driver about the cros_ec_check_features
>> (implemented in platform driver cros_ec_dev), this doesn't seems good to me but
>> I might be wrong. So please, let me know which option do you prefer and if it's
>> the case we will need to change I'll try to do it.
>>
>> Note that I think that a similar use case is used in
>> drivers/iio/common/ssp_sensors/ssp_dev.c:535, where the iio driver registers the
>> sensors to the mfd.
> 
> It would be advantageous to avoid a web of inter-subsystem calls to
> register devices.  I think I could bear calls to mfd_add_* from
> drivers/platform, as the two subsystems are fairly interchangeable,
> and it does have the added benefit of saving duplication of the device
> registering code.  Calling mfd_add_* from IIO seems plain wrong though.
> 
> A better solution however and one that we've utilised in the past is
> to have the MFD drivers call into the platform (i.e. drivers/platform)
> drivers to see if certain devices are available.  Is this possible in
> your use-case?
> 

So just to be clear before I start to work on it. What you want is I get rid of
the MFD stuff from drivers/platform/chrome/cros_ec_dev and move to
drivers/mfd/cros_ec.c. The platform/chrome driver should publish how to check
the features and leave the mfd/cros_ec driver add the MFD subdevices.

For this patch series this means get rid of:

drivers/platform/chrome/cros_ec_dev.c:412:static const struct mfd_cell
cros_ec_rtc_devs[] ...

drivers/platform/chrome/cros_ec_dev.c:404: ret = mfd_add_devices(ec->dev,
0,cros_ec_rtc_devs ...

As I said the sensors subdevice (which is already upstream) uses the same
approach, so I think you will also want do the same for the sensors subdevice?
Sounds good if first we try to land the RTC part and the I'll prepare a patch
series to fix the sensors hub part?

> NB: I've just had a look at the SSP IIO driver and I have a number of
> problems with it.  You should not be using that as a good example of
> why mfd_add_* should be used outside of MFD.
> 

Yeah, I did not try to use the SSP IIO driver as a good example to justify
myself, I just wasted let you know that the approach we used is also used in
other cases, thus if the approach is wrong we should have in mind that we should
also fix the SSP IIO driver. Just this.

Thanks,
 Enric

>>>>  static int ec_device_probe(struct platform_device *pdev)
>>>>  {
>>>>  	int retval = -ENOMEM;
>>>> @@ -441,6 +459,10 @@ static int ec_device_probe(struct platform_device *pdev)
>>>>  	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
>>>>  		cros_ec_sensors_register(ec);
>>>>  
>>>> +	/* check whether this EC instance has RTC host command support */
>>>> +	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
>>>> +		cros_ec_rtc_register(ec);
>>>> +
>>>>  	return 0;
>>>>  
>>>>  dev_reg_failed:
>>>
> 

[rtc-linux] Re: [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Enric Balletbo i Serra]

Hi Lee,

On 15/03/17 11:24, Lee Jones wrote:
> On Tue, 14 Mar 2017, Enric Balletbo i Serra wrote:
>> On 14/03/17 14:59, Lee Jones wrote:
>>> On Tue, 14 Feb 2017, Enric Balletbo i Serra wrote:
>>>
>>>> From: Stephen Barber <smbarber@chromium.org>
>>>>
>>>> If the EC supports RTC host commands, expose an RTC device.
>>>>
>>>> Signed-off-by: Stephen Barber <smbarber@chromium.org>
>>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>>>> Acked-by: Benson Leung <bleung@chromium.org>
>>>> ---
>>>> Changes since v2:
>>>>  - Acked by Benson Leung
>>>> Changes since v1:
>>>>  - none
>>>>
>>>>  drivers/platform/chrome/cros_ec_dev.c | 22 ++++++++++++++++++++++
>>>>  1 file changed, 22 insertions(+)
>>>>
>>>> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
>>>> index 47268ec..ebe029d 100644
>>>> --- a/drivers/platform/chrome/cros_ec_dev.c
>>>> +++ b/drivers/platform/chrome/cros_ec_dev.c
>>>> @@ -383,6 +383,24 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
>>>>  	kfree(msg);
>>>>  }
>>>>  
>>>> +static const struct mfd_cell cros_ec_rtc_devs[] = {
>>>> +	{
>>>> +		.name = "cros-ec-rtc",
>>>> +		.id   = -1,
>>>> +	},
>>>> +};
>>>> +
>>>> +static void cros_ec_rtc_register(struct cros_ec_dev *ec)
>>>> +{
>>>> +	int ret;
>>>> +
>>>> +	ret = mfd_add_devices(ec->dev, 0, cros_ec_rtc_devs,
>>>> +			      ARRAY_SIZE(cros_ec_rtc_devs),
>>>> +			      NULL, 0, NULL);
>>>> +	if (ret)
>>>> +		dev_err(ec->dev, "failed to add cros-ec-rtc device: %d\n", ret);
>>>> +}
>>>
>>> Holey poop!  Why are you using the MFD API outside of MFD?
>>>
>>> Why can't you register this from the MFD driver?
>>>
>>
>> Actually the MFD doesn't know how to check if this feature is available or not,
>> instead is the platform driver cros_ec_dev who knows how to check this and if it
>> exists adds the rtc device.
>>
>> if (cros_ec_check_features(ec, EC_FEATURE_RTC))
>> 	cros_ec_rtc_register(ec); /* add the mfd device */
>>
>> Same approach was used in the same file for the Sensors Hub (already upstream). See:
>>
>>   drivers/platform/chrome/cros_ec_dev.c:462
>>   drivers/platform/chrome/cros_ec_dev.c:372
>>
>> I didn't know that the MFD API was restricted outside MFD. In such case what I
>> need to do is let know the MFD driver about the cros_ec_check_features
>> (implemented in platform driver cros_ec_dev), this doesn't seems good to me but
>> I might be wrong. So please, let me know which option do you prefer and if it's
>> the case we will need to change I'll try to do it.
>>
>> Note that I think that a similar use case is used in
>> drivers/iio/common/ssp_sensors/ssp_dev.c:535, where the iio driver registers the
>> sensors to the mfd.
> 
> It would be advantageous to avoid a web of inter-subsystem calls to
> register devices.  I think I could bear calls to mfd_add_* from
> drivers/platform, as the two subsystems are fairly interchangeable,
> and it does have the added benefit of saving duplication of the device
> registering code.  Calling mfd_add_* from IIO seems plain wrong though.
> 
> A better solution however and one that we've utilised in the past is
> to have the MFD drivers call into the platform (i.e. drivers/platform)
> drivers to see if certain devices are available.  Is this possible in
> your use-case?
> 

So just to be clear before I start to work on it. What you want is I get rid of
the MFD stuff from drivers/platform/chrome/cros_ec_dev and move to
drivers/mfd/cros_ec.c. The platform/chrome driver should publish how to check
the features and leave the mfd/cros_ec driver add the MFD subdevices.

For this patch series this means get rid of:

drivers/platform/chrome/cros_ec_dev.c:412:static const struct mfd_cell
cros_ec_rtc_devs[] ...

drivers/platform/chrome/cros_ec_dev.c:404: ret = mfd_add_devices(ec->dev,
0,cros_ec_rtc_devs ...

As I said the sensors subdevice (which is already upstream) uses the same
approach, so I think you will also want do the same for the sensors subdevice?
Sounds good if first we try to land the RTC part and the I'll prepare a patch
series to fix the sensors hub part?

> NB: I've just had a look at the SSP IIO driver and I have a number of
> problems with it.  You should not be using that as a good example of
> why mfd_add_* should be used outside of MFD.
> 

Yeah, I did not try to use the SSP IIO driver as a good example to justify
myself, I just wasted let you know that the approach we used is also used in
other cases, thus if the approach is wrong we should have in mind that we should
also fix the SSP IIO driver. Just this.

Thanks,
 Enric

>>>>  static int ec_device_probe(struct platform_device *pdev)
>>>>  {
>>>>  	int retval = -ENOMEM;
>>>> @@ -441,6 +459,10 @@ static int ec_device_probe(struct platform_device *pdev)
>>>>  	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
>>>>  		cros_ec_sensors_register(ec);
>>>>  
>>>> +	/* check whether this EC instance has RTC host command support */
>>>> +	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
>>>> +		cros_ec_rtc_register(ec);
>>>> +
>>>>  	return 0;
>>>>  
>>>>  dev_reg_failed:
>>>
> 

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Lee Jones]

On Wed, 15 Mar 2017, Enric Balletbo i Serra wrote:

> Hi Lee,
> 
> On 15/03/17 11:24, Lee Jones wrote:
> > On Tue, 14 Mar 2017, Enric Balletbo i Serra wrote:
> >> On 14/03/17 14:59, Lee Jones wrote:
> >>> On Tue, 14 Feb 2017, Enric Balletbo i Serra wrote:
> >>>
> >>>> From: Stephen Barber <smbarber@chromium.org>
> >>>>
> >>>> If the EC supports RTC host commands, expose an RTC device.
> >>>>
> >>>> Signed-off-by: Stephen Barber <smbarber@chromium.org>
> >>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >>>> Acked-by: Benson Leung <bleung@chromium.org>
> >>>> ---
> >>>> Changes since v2:
> >>>>  - Acked by Benson Leung
> >>>> Changes since v1:
> >>>>  - none
> >>>>
> >>>>  drivers/platform/chrome/cros_ec_dev.c | 22 ++++++++++++++++++++++
> >>>>  1 file changed, 22 insertions(+)
> >>>>
> >>>> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
> >>>> index 47268ec..ebe029d 100644
> >>>> --- a/drivers/platform/chrome/cros_ec_dev.c
> >>>> +++ b/drivers/platform/chrome/cros_ec_dev.c
> >>>> @@ -383,6 +383,24 @@ static void cros_ec_sensors_register(struct cros_ec_dev *ec)
> >>>>  	kfree(msg);
> >>>>  }
> >>>>  
> >>>> +static const struct mfd_cell cros_ec_rtc_devs[] = {
> >>>> +	{
> >>>> +		.name = "cros-ec-rtc",
> >>>> +		.id   = -1,
> >>>> +	},
> >>>> +};
> >>>> +
> >>>> +static void cros_ec_rtc_register(struct cros_ec_dev *ec)
> >>>> +{
> >>>> +	int ret;
> >>>> +
> >>>> +	ret = mfd_add_devices(ec->dev, 0, cros_ec_rtc_devs,
> >>>> +			      ARRAY_SIZE(cros_ec_rtc_devs),
> >>>> +			      NULL, 0, NULL);
> >>>> +	if (ret)
> >>>> +		dev_err(ec->dev, "failed to add cros-ec-rtc device: %d\n", ret);
> >>>> +}
> >>>
> >>> Holey poop!  Why are you using the MFD API outside of MFD?
> >>>
> >>> Why can't you register this from the MFD driver?
> >>>
> >>
> >> Actually the MFD doesn't know how to check if this feature is available or not,
> >> instead is the platform driver cros_ec_dev who knows how to check this and if it
> >> exists adds the rtc device.
> >>
> >> if (cros_ec_check_features(ec, EC_FEATURE_RTC))
> >> 	cros_ec_rtc_register(ec); /* add the mfd device */
> >>
> >> Same approach was used in the same file for the Sensors Hub (already upstream). See:
> >>
> >>   drivers/platform/chrome/cros_ec_dev.c:462
> >>   drivers/platform/chrome/cros_ec_dev.c:372
> >>
> >> I didn't know that the MFD API was restricted outside MFD. In such case what I
> >> need to do is let know the MFD driver about the cros_ec_check_features
> >> (implemented in platform driver cros_ec_dev), this doesn't seems good to me but
> >> I might be wrong. So please, let me know which option do you prefer and if it's
> >> the case we will need to change I'll try to do it.
> >>
> >> Note that I think that a similar use case is used in
> >> drivers/iio/common/ssp_sensors/ssp_dev.c:535, where the iio driver registers the
> >> sensors to the mfd.
> > 
> > It would be advantageous to avoid a web of inter-subsystem calls to
> > register devices.  I think I could bear calls to mfd_add_* from
> > drivers/platform, as the two subsystems are fairly interchangeable,
> > and it does have the added benefit of saving duplication of the device
> > registering code.  Calling mfd_add_* from IIO seems plain wrong though.
> > 
> > A better solution however and one that we've utilised in the past is
> > to have the MFD drivers call into the platform (i.e. drivers/platform)
> > drivers to see if certain devices are available.  Is this possible in
> > your use-case?
> > 
> 
> So just to be clear before I start to work on it. What you want is I get rid of
> the MFD stuff from drivers/platform/chrome/cros_ec_dev and move to
> drivers/mfd/cros_ec.c. The platform/chrome driver should publish how to check
> the features and leave the mfd/cros_ec driver add the MFD subdevices.
> 
> For this patch series this means get rid of:
> 
> drivers/platform/chrome/cros_ec_dev.c:412:static const struct mfd_cell
> cros_ec_rtc_devs[] ...
> 
> drivers/platform/chrome/cros_ec_dev.c:404: ret = mfd_add_devices(ec->dev,
> 0,cros_ec_rtc_devs ...
> 
> As I said the sensors subdevice (which is already upstream) uses the same
> approach, so I think you will also want do the same for the sensors subdevice?
> Sounds good if first we try to land the RTC part and the I'll prepare a patch
> series to fix the sensors hub part?

You got it.  Sounds good.

> > NB: I've just had a look at the SSP IIO driver and I have a number of
> > problems with it.  You should not be using that as a good example of
> > why mfd_add_* should be used outside of MFD.
> > 
> 
> Yeah, I did not try to use the SSP IIO driver as a good example to justify
> myself, I just wasted let you know that the approach we used is also used in
> other cases, thus if the approach is wrong we should have in mind that we should
> also fix the SSP IIO driver. Just this.
> 
> Thanks,
>  Enric
> 
> >>>>  static int ec_device_probe(struct platform_device *pdev)
> >>>>  {
> >>>>  	int retval = -ENOMEM;
> >>>> @@ -441,6 +459,10 @@ static int ec_device_probe(struct platform_device *pdev)
> >>>>  	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
> >>>>  		cros_ec_sensors_register(ec);
> >>>>  
> >>>> +	/* check whether this EC instance has RTC host command support */
> >>>> +	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
> >>>> +		cros_ec_rtc_register(ec);
> >>>> +
> >>>>  	return 0;
> >>>>  
> >>>>  dev_reg_failed:
> >>>
> > 

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[rtc-linux] Re: [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Lee Jones]

On Wed, 15 Mar 2017, Enric Balletbo i Serra wrote:

> Hi Lee,
>=20
> On 15/03/17 11:24, Lee Jones wrote:
> > On Tue, 14 Mar 2017, Enric Balletbo i Serra wrote:
> >> On 14/03/17 14:59, Lee Jones wrote:
> >>> On Tue, 14 Feb 2017, Enric Balletbo i Serra wrote:
> >>>
> >>>> From: Stephen Barber <smbarber@chromium.org>
> >>>>
> >>>> If the EC supports RTC host commands, expose an RTC device.
> >>>>
> >>>> Signed-off-by: Stephen Barber <smbarber@chromium.org>
> >>>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> >>>> Acked-by: Benson Leung <bleung@chromium.org>
> >>>> ---
> >>>> Changes since v2:
> >>>>  - Acked by Benson Leung
> >>>> Changes since v1:
> >>>>  - none
> >>>>
> >>>>  drivers/platform/chrome/cros_ec_dev.c | 22 ++++++++++++++++++++++
> >>>>  1 file changed, 22 insertions(+)
> >>>>
> >>>> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platfor=
m/chrome/cros_ec_dev.c
> >>>> index 47268ec..ebe029d 100644
> >>>> --- a/drivers/platform/chrome/cros_ec_dev.c
> >>>> +++ b/drivers/platform/chrome/cros_ec_dev.c
> >>>> @@ -383,6 +383,24 @@ static void cros_ec_sensors_register(struct cro=
s_ec_dev *ec)
> >>>>  	kfree(msg);
> >>>>  }
> >>>> =20
> >>>> +static const struct mfd_cell cros_ec_rtc_devs[] =3D {
> >>>> +	{
> >>>> +		.name =3D "cros-ec-rtc",
> >>>> +		.id   =3D -1,
> >>>> +	},
> >>>> +};
> >>>> +
> >>>> +static void cros_ec_rtc_register(struct cros_ec_dev *ec)
> >>>> +{
> >>>> +	int ret;
> >>>> +
> >>>> +	ret =3D mfd_add_devices(ec->dev, 0, cros_ec_rtc_devs,
> >>>> +			      ARRAY_SIZE(cros_ec_rtc_devs),
> >>>> +			      NULL, 0, NULL);
> >>>> +	if (ret)
> >>>> +		dev_err(ec->dev, "failed to add cros-ec-rtc device: %d\n", ret);
> >>>> +}
> >>>
> >>> Holey poop!  Why are you using the MFD API outside of MFD?
> >>>
> >>> Why can't you register this from the MFD driver?
> >>>
> >>
> >> Actually the MFD doesn't know how to check if this feature is availabl=
e or not,
> >> instead is the platform driver cros_ec_dev who knows how to check this=
 and if it
> >> exists adds the rtc device.
> >>
> >> if (cros_ec_check_features(ec, EC_FEATURE_RTC))
> >> 	cros_ec_rtc_register(ec); /* add the mfd device */
> >>
> >> Same approach was used in the same file for the Sensors Hub (already u=
pstream). See:
> >>
> >>   drivers/platform/chrome/cros_ec_dev.c:462
> >>   drivers/platform/chrome/cros_ec_dev.c:372
> >>
> >> I didn't know that the MFD API was restricted outside MFD. In such cas=
e what I
> >> need to do is let know the MFD driver about the cros_ec_check_features
> >> (implemented in platform driver cros_ec_dev), this doesn't seems good =
to me but
> >> I might be wrong. So please, let me know which option do you prefer an=
d if it's
> >> the case we will need to change I'll try to do it.
> >>
> >> Note that I think that a similar use case is used in
> >> drivers/iio/common/ssp_sensors/ssp_dev.c:535, where the iio driver reg=
isters the
> >> sensors to the mfd.
> >=20
> > It would be advantageous to avoid a web of inter-subsystem calls to
> > register devices.  I think I could bear calls to mfd_add_* from
> > drivers/platform, as the two subsystems are fairly interchangeable,
> > and it does have the added benefit of saving duplication of the device
> > registering code.  Calling mfd_add_* from IIO seems plain wrong though.
> >=20
> > A better solution however and one that we've utilised in the past is
> > to have the MFD drivers call into the platform (i.e. drivers/platform)
> > drivers to see if certain devices are available.  Is this possible in
> > your use-case?
> >=20
>=20
> So just to be clear before I start to work on it. What you want is I get =
rid of
> the MFD stuff from drivers/platform/chrome/cros_ec_dev and move to
> drivers/mfd/cros_ec.c. The platform/chrome driver should publish how to c=
heck
> the features and leave the mfd/cros_ec driver add the MFD subdevices.
>=20
> For this patch series this means get rid of:
>=20
> drivers/platform/chrome/cros_ec_dev.c:412:static const struct mfd_cell
> cros_ec_rtc_devs[] ...
>=20
> drivers/platform/chrome/cros_ec_dev.c:404: ret =3D mfd_add_devices(ec->de=
v,
> 0,cros_ec_rtc_devs ...
>=20
> As I said the sensors subdevice (which is already upstream) uses the same
> approach, so I think you will also want do the same for the sensors subde=
vice?
> Sounds good if first we try to land the RTC part and the I'll prepare a p=
atch
> series to fix the sensors hub part?

You got it.  Sounds good.

> > NB: I've just had a look at the SSP IIO driver and I have a number of
> > problems with it.  You should not be using that as a good example of
> > why mfd_add_* should be used outside of MFD.
> >=20
>=20
> Yeah, I did not try to use the SSP IIO driver as a good example to justif=
y
> myself, I just wasted let you know that the approach we used is also used=
 in
> other cases, thus if the approach is wrong we should have in mind that we=
 should
> also fix the SSP IIO driver. Just this.
>=20
> Thanks,
>  Enric
>=20
> >>>>  static int ec_device_probe(struct platform_device *pdev)
> >>>>  {
> >>>>  	int retval =3D -ENOMEM;
> >>>> @@ -441,6 +459,10 @@ static int ec_device_probe(struct platform_devi=
ce *pdev)
> >>>>  	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
> >>>>  		cros_ec_sensors_register(ec);
> >>>> =20
> >>>> +	/* check whether this EC instance has RTC host command support */
> >>>> +	if (cros_ec_check_features(ec, EC_FEATURE_RTC))
> >>>> +		cros_ec_rtc_register(ec);
> >>>> +
> >>>>  	return 0;
> >>>> =20
> >>>>  dev_reg_failed:
> >>>
> >=20

--=20
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org =E2=94=82 Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[rtc-linux] Re: [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Tracy Smith]

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

Hi, not sure if this is the correct venue to post a question, so please
forgive and direct me to the correct board or list if not.

A custom board implementation using a ARM-8 Cortex A53 NXP LS1043ardb is
considering moving the RTC from the control board to an FPGA.  Reason is
for accuracy of time/date and efficiency for their FPGA/DSP application.

1) What kernel changes would be required to support such an implementation
and these would need to be pushed to the community under the GPL?

2) Is it a trivial task to make such changes in the kernel to support an
RTC not located on the control board/SoC itself?

3) What is the expected behavior of the kernel with no changes added to
accommodate such an architecture?

4) Is there a configurable kernel option to allow for such a change, or
have these or similar changes already been added to the kernel?

thx,
Tracy

-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

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

[PATCH v3 0/4] mfd: cros-ec: Some fixes and improvements.

[Enric Balletbo i Serra]

Dear all,

Basically this is a resend and rebase due that [2] and [3] are currently
merged so all dependecies are in mainline now.

To remmember:

* 1/4 mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.

As pointed by Lee Jones in this thread [1] we should not use the MFD API
outside of MFD. For this reason the cros-ec-rtc did not get accepted yet.
The reality is that we are calling mfd_add_devices from cros-ec-dev driver
already, so this patch get rid off the MFD calls inside the chardev driver
and moves to cros-ec MFD. Also I think the chardev device should simply
implement the ioctl calls to access to it from userspace.

The above patch involves MFD, IIO and platform chrome subsystems.

* 2/4 mfd: cros_ec: Introduce RTC commands and events definitions
* 3/4 rtc: cros-ec: add cros-ec-rtc driver
* 4/4 mfd: cros_ec: add RTC as mfd subdevice

These patches are the cros-ec RTC driver, 3 and 4 patches are already
acked by the subsystem maintainers involved and are equal to the last
version I send. Patch 5 registers the rtc cell inside the cros-ec MFD
intead of cros-ec-dev chardev driver.

Changes since v2:
- Rebase on top of mainline.
- Removed patch 'mfd: cros-ec: Fix host command buffer size' from series
as was already picked.

Changes since v1:
- Removed patch 'iio: cros_ec_sensors: Fix return value to get raw and
calibbias data' from series as was already picked.
- Removed patch 'iio: cros_ec_sensors: Fix return value to get raw and
calibbias data' from series as was already picked.
- Patch 2/5: Acked-by: Jonathan Cameron <***@kernel.org>

[1] https://www.spinics.net/lists/kernel/msg2465099.html
[2] https://lkml.org/lkml/2017/3/17/319
[3] https://lkml.org/lkml/2017/3/17/321

Best regards,

Enric Balletbo i Serra (1):
  mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.

Stephen Barber (3):
  mfd: cros_ec: Introduce RTC commands and events definitions.
  rtc: cros-ec: add cros-ec-rtc driver.
  mfd: cros_ec: add RTC as mfd subdevice

 .../iio/common/cros_ec_sensors/cros_ec_sensors.c   |   8 -
 .../common/cros_ec_sensors/cros_ec_sensors_core.c  |   8 +-
 drivers/iio/light/cros_ec_light_prox.c             |   8 -
 drivers/iio/pressure/cros_ec_baro.c                |   8 -
 drivers/mfd/cros_ec.c                              | 178 +++++++++
 drivers/platform/chrome/cros_ec_dev.c              | 161 --------
 drivers/rtc/Kconfig                                |  10 +
 drivers/rtc/Makefile                               |   1 +
 drivers/rtc/rtc-cros-ec.c                          | 412 +++++++++++++++++++++
 include/linux/mfd/cros_ec.h                        |   6 +-
 include/linux/mfd/cros_ec_commands.h               |   8 +
 11 files changed, 619 insertions(+), 189 deletions(-)
 create mode 100644 drivers/rtc/rtc-cros-ec.c

-- 
2.9.3

[PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Enric Balletbo i Serra]

From: Stephen Barber <smbarber@chromium.org>

If the EC supports RTC host commands, expose an RTC device.

Signed-off-by: Stephen Barber <smbarber@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/mfd/cros_ec.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index 75a27a6..ff972fb 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -51,6 +51,10 @@ static const struct mfd_cell ec_pd_cell = {
 	.pdata_size = sizeof(pd_p),
 };
 
+static const struct mfd_cell ec_rtc_cell = {
+	.name = "cros-ec-rtc",
+};
+
 static irqreturn_t ec_irq_thread(int irq, void *data)
 {
 	struct cros_ec_device *ec_dev = data;
@@ -245,6 +249,16 @@ static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
 	kfree(msg);
 }
 
+static void cros_ec_rtc_register(struct cros_ec_device *ec_dev)
+{
+	int ret;
+
+	ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_rtc_cell,
+			      1, NULL, 0, NULL);
+	if (ret)
+		dev_err(ec_dev->dev, "failed to add EC RTC\n");
+}
+
 int cros_ec_register(struct cros_ec_device *ec_dev)
 {
 	struct device *dev = ec_dev->dev;
@@ -294,6 +308,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 	if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
 		cros_ec_sensors_register(ec_dev);
 
+	/* Check whether this EC has RTC support */
+	if (cros_ec_check_features(ec_dev, EC_FEATURE_RTC))
+		cros_ec_rtc_register(ec_dev);
+
 	if (ec_dev->max_passthru) {
 		/*
 		 * Register a PD device as well on top of this device.
-- 
2.9.3

Re: [PATCH v3 4/4] mfd: cros_ec: add RTC as mfd subdevice

[Lee Jones]

On Wed, 12 Jul 2017, Enric Balletbo i Serra wrote:

> From: Stephen Barber <smbarber@chromium.org>
> 
> If the EC supports RTC host commands, expose an RTC device.
> 
> Signed-off-by: Stephen Barber <smbarber@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>  drivers/mfd/cros_ec.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index 75a27a6..ff972fb 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -51,6 +51,10 @@ static const struct mfd_cell ec_pd_cell = {
>  	.pdata_size = sizeof(pd_p),
>  };
>  
> +static const struct mfd_cell ec_rtc_cell = {
> +	.name = "cros-ec-rtc",
> +};
> +
>  static irqreturn_t ec_irq_thread(int irq, void *data)
>  {
>  	struct cros_ec_device *ec_dev = data;
> @@ -245,6 +249,16 @@ static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
>  	kfree(msg);
>  }
>  
> +static void cros_ec_rtc_register(struct cros_ec_device *ec_dev)
> +{
> +	int ret;
> +
> +	ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_rtc_cell,
> +			      1, NULL, 0, NULL);
> +	if (ret)
> +		dev_err(ec_dev->dev, "failed to add EC RTC\n");
> +}

Why do you need a new function just to call a single function?

And by making it void, you're also loosing the return value.

If mfd_add_devices() fails, something has gone wrong!

>  int cros_ec_register(struct cros_ec_device *ec_dev)
>  {
>  	struct device *dev = ec_dev->dev;
> @@ -294,6 +308,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  	if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
>  		cros_ec_sensors_register(ec_dev);
>  
> +	/* Check whether this EC has RTC support */
> +	if (cros_ec_check_features(ec_dev, EC_FEATURE_RTC))
> +		cros_ec_rtc_register(ec_dev);
> +
>  	if (ec_dev->max_passthru) {
>  		/*
>  		 * Register a PD device as well on top of this device.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog