linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/9] platform/chrome: rtc: Add support for Wilco EC
@ 2019-01-19  0:14 Nick Crews
  2019-01-19  0:14 ` [PATCH v3 5/9] platform/chrome: rtc: Add RTC driver Nick Crews
  2019-01-22 15:26 ` [PATCH v3 0/9] platform/chrome: rtc: Add support for Wilco EC Enric Balletbo Serra
  0 siblings, 2 replies; 4+ messages in thread
From: Nick Crews @ 2019-01-19  0:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: groeck, sjg, djkurtz, dlaurie, Nick Crews, linux-rtc,
	Enric Balletbo i Serra, Alessandro Zummo, Benson Leung,
	Nick Crews, Duncan Laurie, Alexandre Belloni


There is a new chromebook that contains a different Embedded Controller
(codename Wilco) than the rest of the chromebook series. Thus the kernel
requires a different driver than the already existing and generalized
cros_ec_* drivers. Specifically, this driver adds support for getting
and setting the RTC on the EC, adding a binary sysfs attribute
that receives ACPI events from the EC, adding a binary sysfs
attribute to request telemetry data from the EC (useful for enterprise
applications), adding a debugfs interface for sending/receiving raw byte
sequesnces to the EC, and adding normal sysfs attributes to get/set various
other properties on the EC. The core of the communication with the EC
is implemented in wilco_ec/mailbox.c, using a simple byte-level protocol
with a checksum, transmitted over an eSPI bus. For debugging purposes,
a raw attribute is also provided which can write/read arbitrary
bytes to/from the eSPI bus.

We attempted to adhere to the sysfs principles of "one piece of data per
attribute" as much as possible, and mostly succeded. However, with the
wilco_ec/adv_power.h attributes, which deal with scheduling power usage,
we found it most elegant to bundle setting event times for an entire day
into a single attribute, so at most you are using attributes formatted
as "%d %d %d %d %d %d". With the telemetry attribute, we had to use a
binary attribute, instead of the preferable human-readable ascii, in
order to keep secure the information which is proprietary to the
enterprise service provider. This opaque binary data will be read and
sent using a proprietary daemon running on the OS.

The RTC driver is exposed as a standard RTC driver with
read/write functionality.

For event notification, the Wilco EC can return extended events that
are not handled by standard ACPI objects. These events can
include hotkeys which map to standard functions like brightness
controls, or information about EC controlled features like the
charger or battery. These events are triggered with an
ACPI Notify(0x90) and the event data buffer is read through an ACPI
method provided by the BIOS which reads the event buffer from EC RAM.
These events are then processed, with hotkey events being sent
to the input subsystem and other events put into a queue which
can be read by a userspace daemon via a sysfs attribute.

The rest of the attributes are categorized as either "properties" or
"legacy". "legacy" implies that the attribute existed on the EC before it
was modified for ChromeOS, and "properties" implies that the attribute
exposes functionality that was added to the EC specifically for
ChromeOS. They are mostly boolean flags or percentages.

A full thread of the development of these patches can be found at
https://chromium-review.googlesource.com/c/1371034. This thread contains
comments and revisions that could be helpful in understanding how the
driver arrived at the state it is in now. The thread also contains some
ChromeOS specific patches that actually enable the driver. If you want
to test the patch yourself, you would have to install the ChromeOS SDK
and cherry pick in these patches.

I also wrote some integration tests using the Tast testing framework that
ChromeOS uses. It would require a full ChromeOS SDK to actually run the
tests, but the source of the tests, written in Go, are useful for
understanding what the desired behavior is. You can view the tests here:
https://chromium-review.googlesource.com/c/1372575

Thank you for your comments!

Changes in v3:
- Change <= to >= in mec_in_range()
- Add " - EC_HOST_CMD_REGION0" to offset arg for io_bytes_mec()
- remove unused ret in probe()
- Add newline spacer in probe()
- rm unnecessary res in get_resource()
- s/8bit/8-bit
- rm first sleep when sending command to EC
- Move the attribute to the debugfs system
- Move the implementation to debugfs.c
- Improve the raw hex parsing
- Encapsulate global variables in one object
- Add safety check when passing less than 3 bytes
- Move documentation to debugfs-wilco-ec
- explicitly define toplevel_groups from the start,
so adding telem later makes sense
- Break version attribute into individual attributes
- rm unused WILCO_EC_ATTR_RW macro
- Moved some #defines from legacy.h to legacy.c
- rm #define for driver name
- Don't compute weekday when reading from RTC.
  Still set weekday when writing, as RTC needs this
  to control advanced power scheduling
- rm check for invalid month data
- Set range_min and range_max on rtc_device
- change err check from "if (ret < 0)" to "if (ret)"
- Now bubble up error codes from within sysfs_init()
- Add comment that PID means Property ID
- rm some useless references to internal docs from documentation
- add err check on returned data size
- add check on read request offset and size

Changes in v2:
- Fixed kernel-doc comments
- Fixed include of linux/mfd/cros_ec_lpc_mec.h
- cros_ec_lpc_mec_in_range() returns -EINVAL on error
- Added parens around macro variables
- Remove COMPILE_TEST from Kconfig because inb()/outb()
won't work on anything but X86
- Add myself as module author
- Tweak mailbox()
- Add sysfs documentation
- rm duplicate EC_MAILBOX_DATA_SIZE defs
- Make docstrings follow kernel style
- Fix tags in commit msg
- Move Kconfig to subdirectory
- Reading raw now includes ASCII translation
- Remove license boiler plate
- Remove "wilco_ec_sysfs -" docstring prefix
- Fix accidental Makefile deletion
- Add documentation for sysfs entries
- Change "enable ? 0 : 1" to "!enable"
- No longer override error code from sysfs_init()
- Put attributes in the legacy file to begin with, don't move later
- Remove duplicate error messages when init()ing sysfs
- rm license boiler plate
- rm "wilco_ec_rtc -" prefix in docstring
- Make rtc driver its own module within the drivers/rtc/ directory
- Register a rtc device from core.c that is picked up by this driver
- rm "wilco_ec_event -" prefix from docstring
- rm license boiler plate
- Add sysfs directory documentation
- Fix cosmetics
- events are init()ed before subdrivers now
- rm license boiler plate
- rm "wilco_ec_properties -" prefix from docstring
- Add documentation
- rm license boiler plate
- rm "wilco_ec_adv_power - " prefix from docstring
- Add documentation
- make format strings in read() and store() functions static

Duncan Laurie (6):
  platform/chrome: Remove cros_ec dependency in lpc_mec
  platform/chrome: Add new driver for Wilco EC
  platform/chrome: Add support for raw commands in debugfs
  platform/chrome: Add sysfs attributes
  platform/chrome: rtc: Add RTC driver
  platform/chrome: Add event handling

Nick Crews (3):
  platform/chrome: Add EC properties
  platform/chrome: Add peakshift and adv_batt_charging
  platform/chrome: Add binary telemetry attributes

 Documentation/ABI/testing/debugfs-wilco-ec    |  23 +
 .../ABI/testing/sysfs-platform-wilco-ec       | 196 +++++++
 drivers/platform/chrome/Kconfig               |   4 +-
 drivers/platform/chrome/Makefile              |   2 +
 drivers/platform/chrome/cros_ec_lpc_mec.c     |  52 +-
 drivers/platform/chrome/cros_ec_lpc_mec.h     |  43 +-
 drivers/platform/chrome/cros_ec_lpc_reg.c     |  47 +-
 drivers/platform/chrome/wilco_ec/Kconfig      |  33 ++
 drivers/platform/chrome/wilco_ec/Makefile     |   6 +
 drivers/platform/chrome/wilco_ec/adv_power.c  | 544 ++++++++++++++++++
 drivers/platform/chrome/wilco_ec/adv_power.h  | 183 ++++++
 drivers/platform/chrome/wilco_ec/core.c       | 154 +++++
 drivers/platform/chrome/wilco_ec/debugfs.c    | 218 +++++++
 drivers/platform/chrome/wilco_ec/event.c      | 347 +++++++++++
 drivers/platform/chrome/wilco_ec/legacy.c     | 103 ++++
 drivers/platform/chrome/wilco_ec/legacy.h     |  79 +++
 drivers/platform/chrome/wilco_ec/mailbox.c    | 236 ++++++++
 drivers/platform/chrome/wilco_ec/properties.c | 344 +++++++++++
 drivers/platform/chrome/wilco_ec/properties.h | 180 ++++++
 drivers/platform/chrome/wilco_ec/sysfs.c      | 245 ++++++++
 drivers/platform/chrome/wilco_ec/telemetry.c  |  73 +++
 drivers/platform/chrome/wilco_ec/telemetry.h  |  41 ++
 drivers/platform/chrome/wilco_ec/util.h       |  38 ++
 drivers/rtc/Kconfig                           |  11 +
 drivers/rtc/Makefile                          |   1 +
 drivers/rtc/rtc-wilco-ec.c                    | 177 ++++++
 include/linux/platform_data/wilco-ec.h        | 189 ++++++
 27 files changed, 3509 insertions(+), 60 deletions(-)
 create mode 100644 Documentation/ABI/testing/debugfs-wilco-ec
 create mode 100644 Documentation/ABI/testing/sysfs-platform-wilco-ec
 create mode 100644 drivers/platform/chrome/wilco_ec/Kconfig
 create mode 100644 drivers/platform/chrome/wilco_ec/Makefile
 create mode 100644 drivers/platform/chrome/wilco_ec/adv_power.c
 create mode 100644 drivers/platform/chrome/wilco_ec/adv_power.h
 create mode 100644 drivers/platform/chrome/wilco_ec/core.c
 create mode 100644 drivers/platform/chrome/wilco_ec/debugfs.c
 create mode 100644 drivers/platform/chrome/wilco_ec/event.c
 create mode 100644 drivers/platform/chrome/wilco_ec/legacy.c
 create mode 100644 drivers/platform/chrome/wilco_ec/legacy.h
 create mode 100644 drivers/platform/chrome/wilco_ec/mailbox.c
 create mode 100644 drivers/platform/chrome/wilco_ec/properties.c
 create mode 100644 drivers/platform/chrome/wilco_ec/properties.h
 create mode 100644 drivers/platform/chrome/wilco_ec/sysfs.c
 create mode 100644 drivers/platform/chrome/wilco_ec/telemetry.c
 create mode 100644 drivers/platform/chrome/wilco_ec/telemetry.h
 create mode 100644 drivers/platform/chrome/wilco_ec/util.h
 create mode 100644 drivers/rtc/rtc-wilco-ec.c
 create mode 100644 include/linux/platform_data/wilco-ec.h

-- 
2.20.1.321.g9e740568ce-goog


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

* [PATCH v3 5/9] platform/chrome: rtc: Add RTC driver
  2019-01-19  0:14 [PATCH v3 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
@ 2019-01-19  0:14 ` Nick Crews
  2019-01-22 14:55   ` Enric Balletbo Serra
  2019-01-22 15:26 ` [PATCH v3 0/9] platform/chrome: rtc: Add support for Wilco EC Enric Balletbo Serra
  1 sibling, 1 reply; 4+ messages in thread
From: Nick Crews @ 2019-01-19  0:14 UTC (permalink / raw)
  To: linux-kernel
  Cc: groeck, sjg, djkurtz, dlaurie, Duncan Laurie, Nick Crews,
	linux-rtc, Enric Balletbo i Serra, Alessandro Zummo,
	Benson Leung, Nick Crews, Alexandre Belloni

From: Duncan Laurie <dlaurie@google.com>

This Embedded Controller has an internal RTC that is exposed
as a standard RTC class driver with read/write functionality.

The driver is added to the drivers/rtc/ so that the maintainer of that
directory will be able to comment on this change, as that maintainer is
the expert on this system. In addition, the driver code is called
indirectly after a corresponding device is registered from core.c,
as opposed to core.c registering the driver callbacks directly.

> hwclock --show --rtc /dev/rtc1
2007-12-31 16:01:20.460959-08:00
> hwclock --systohc --rtc /dev/rtc1
> hwclock --show --rtc /dev/rtc1
2018-11-29 17:08:00.780793-08:00

Signed-off-by: Duncan Laurie <dlaurie@google.com>
Signed-off-by: Nick Crews <ncrews@chromium.org>
---

Changes in v3:
- rm #define for driver name
- Don't compute weekday when reading from RTC.
  Still set weekday when writing, as RTC needs this
  to control advanced power scheduling
- rm check for invalid month data
- Set range_min and range_max on rtc_device

Changes in v2:
- rm license boiler plate
- rm "wilco_ec_rtc -" prefix in docstring
- Make rtc driver its own module within the drivers/rtc/ directory
- Register a rtc device from core.c that is picked up by this driver

 drivers/platform/chrome/wilco_ec/core.c |  17 +++
 drivers/rtc/Kconfig                     |  11 ++
 drivers/rtc/Makefile                    |   1 +
 drivers/rtc/rtc-wilco-ec.c              | 177 ++++++++++++++++++++++++
 4 files changed, 206 insertions(+)
 create mode 100644 drivers/rtc/rtc-wilco-ec.c

diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
index 9b4a25e63b56..dd8b896b3833 100644
--- a/drivers/platform/chrome/wilco_ec/core.c
+++ b/drivers/platform/chrome/wilco_ec/core.c
@@ -34,6 +34,7 @@ static int wilco_ec_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct wilco_ec_device *ec;
+	struct platform_device *child_pdev;
 	int ret;
 
 	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
@@ -76,8 +77,24 @@ static int wilco_ec_probe(struct platform_device *pdev)
 		goto rm_debugfs;
 	}
 
+	/*
+	 * Register a RTC platform device that will get picked up by the RTC
+	 * subsystem. This platform device will get freed when its parent device
+	 * dev is unregistered.
+	 */
+	child_pdev = platform_device_register_data(dev, "rtc-wilco-ec",
+						   PLATFORM_DEVID_AUTO,
+						   NULL, 0);
+	if (IS_ERR(child_pdev)) {
+		dev_err(dev, "Failed to create RTC platform device\n");
+		ret = PTR_ERR(child_pdev);
+		goto remove_sysfs;
+	}
+
 	return 0;
 
+remove_sysfs:
+	wilco_ec_sysfs_remove(ec);
 rm_debugfs:
 	wilco_ec_debugfs_remove(ec);
 destroy_mec:
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 225b0b8516f3..d5063c791515 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1814,4 +1814,15 @@ config RTC_DRV_GOLDFISH
 	  Goldfish is a code name for the virtual platform developed by Google
 	  for Android emulation.
 
+config RTC_DRV_WILCO_EC
+	tristate "Wilco EC RTC"
+	depends on WILCO_EC
+	default m
+	help
+	  If you say yes here, you get read/write support for the Real Time
+	  Clock on the Wilco Embedded Controller (Wilco is a kind of Chromebook)
+
+	  This can also be built as a module. If so, the module will
+	  be named "rtc_wilco_ec".
+
 endif # RTC_CLASS
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index df022d820bee..6255ea78da25 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -172,6 +172,7 @@ obj-$(CONFIG_RTC_DRV_V3020)	+= rtc-v3020.o
 obj-$(CONFIG_RTC_DRV_VR41XX)	+= rtc-vr41xx.o
 obj-$(CONFIG_RTC_DRV_VRTC)	+= rtc-mrst.o
 obj-$(CONFIG_RTC_DRV_VT8500)	+= rtc-vt8500.o
+obj-$(CONFIG_RTC_DRV_WILCO_EC)	+= rtc-wilco-ec.o
 obj-$(CONFIG_RTC_DRV_WM831X)	+= rtc-wm831x.o
 obj-$(CONFIG_RTC_DRV_WM8350)	+= rtc-wm8350.o
 obj-$(CONFIG_RTC_DRV_X1205)	+= rtc-x1205.o
diff --git a/drivers/rtc/rtc-wilco-ec.c b/drivers/rtc/rtc-wilco-ec.c
new file mode 100644
index 000000000000..b721cc3f17f0
--- /dev/null
+++ b/drivers/rtc/rtc-wilco-ec.c
@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * RTC interface for Wilco Embedded Controller with R/W abilities
+ *
+ * Copyright 2018 Google LLC
+ *
+ * The corresponding platform device is typically registered in
+ * drivers/platform/chrome/wilco_ec/core.c
+ */
+
+#include <linux/bcd.h>
+#include <linux/err.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/rtc.h>
+#include <linux/timekeeping.h>
+
+#define EC_COMMAND_CMOS			0x7c
+#define EC_CMOS_TOD_WRITE		0x02
+#define EC_CMOS_TOD_READ		0x08
+
+/**
+ * struct ec_rtc_read - Format of RTC returned by EC.
+ * @second: Second value (0..59)
+ * @minute: Minute value (0..59)
+ * @hour: Hour value (0..23)
+ * @day: Day value (1..31)
+ * @month: Month value (1..12)
+ * @year: Year value (full year % 100)
+ * @century: Century value (full year / 100)
+ *
+ * All values are presented in binary (not BCD).
+ */
+struct ec_rtc_read {
+	u8 second;
+	u8 minute;
+	u8 hour;
+	u8 day;
+	u8 month;
+	u8 year;
+	u8 century;
+} __packed;
+
+/**
+ * struct ec_rtc_write - Format of RTC sent to the EC.
+ * @param: EC_CMOS_TOD_WRITE
+ * @century: Century value (full year / 100)
+ * @year: Year value (full year % 100)
+ * @month: Month value (1..12)
+ * @day: Day value (1..31)
+ * @hour: Hour value (0..23)
+ * @minute: Minute value (0..59)
+ * @second: Second value (0..59)
+ * @weekday: Day of the week (0=Saturday)
+ *
+ * All values are presented in BCD.
+ */
+struct ec_rtc_write {
+	u8 param;
+	u8 century;
+	u8 year;
+	u8 month;
+	u8 day;
+	u8 hour;
+	u8 minute;
+	u8 second;
+	u8 weekday;
+} __packed;
+
+int wilco_ec_rtc_read(struct device *dev, struct rtc_time *tm)
+{
+	struct wilco_ec_device *ec = dev_get_drvdata(dev->parent);
+	u8 param = EC_CMOS_TOD_READ;
+	struct ec_rtc_read rtc;
+	struct wilco_ec_message msg = {
+		.type = WILCO_EC_MSG_LEGACY,
+		.flags = WILCO_EC_FLAG_RAW_RESPONSE,
+		.command = EC_COMMAND_CMOS,
+		.request_data = &param,
+		.request_size = sizeof(param),
+		.response_data = &rtc,
+		.response_size = sizeof(rtc),
+	};
+	int ret;
+
+	ret = wilco_ec_mailbox(ec, &msg);
+	if (ret < 0)
+		return ret;
+
+	tm->tm_sec	= rtc.second;
+	tm->tm_min	= rtc.minute;
+	tm->tm_hour	= rtc.hour;
+	tm->tm_mday	= rtc.day;
+	tm->tm_mon	= rtc.month - 1;
+	tm->tm_year	= rtc.year + (rtc.century * 100) - 1900;
+	tm->tm_yday	= rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year);
+
+	/* Don't compute day of week, we don't need it. */
+	tm->tm_wday = -1;
+
+	return 0;
+}
+
+int wilco_ec_rtc_write(struct device *dev, struct rtc_time *tm)
+{
+	struct wilco_ec_device *ec = dev_get_drvdata(dev->parent);
+	struct ec_rtc_write rtc;
+	struct wilco_ec_message msg = {
+		.type = WILCO_EC_MSG_LEGACY,
+		.flags = WILCO_EC_FLAG_RAW_RESPONSE,
+		.command = EC_COMMAND_CMOS,
+		.request_data = &rtc,
+		.request_size = sizeof(rtc),
+	};
+	int year = tm->tm_year + 1900;
+	/*
+	 * Convert from 0=Sunday to 0=Saturday for the EC
+	 * We DO need to set weekday because the EC controls battery charging
+	 * schedules that depend on the day of the week.
+	 */
+	int wday = tm->tm_wday == 6 ? 0 : tm->tm_wday + 1;
+	int ret;
+
+	rtc.param	= EC_CMOS_TOD_WRITE;
+	rtc.century	= bin2bcd(year / 100);
+	rtc.year	= bin2bcd(year % 100);
+	rtc.month	= bin2bcd(tm->tm_mon + 1);
+	rtc.day		= bin2bcd(tm->tm_mday);
+	rtc.hour	= bin2bcd(tm->tm_hour);
+	rtc.minute	= bin2bcd(tm->tm_min);
+	rtc.second	= bin2bcd(tm->tm_sec);
+	rtc.weekday	= bin2bcd(wday);
+
+	ret = wilco_ec_mailbox(ec, &msg);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+static const struct rtc_class_ops wilco_ec_rtc_ops = {
+	.read_time = wilco_ec_rtc_read,
+	.set_time = wilco_ec_rtc_write,
+};
+
+static int wilco_ec_rtc_probe(struct platform_device *pdev)
+{
+	struct rtc_device *rtc;
+
+	rtc = devm_rtc_allocate_device(&pdev->dev);
+	if (IS_ERR(rtc))
+		return PTR_ERR(rtc);
+
+	rtc->ops = &wilco_ec_rtc_ops;
+	/* EC only supports this century */
+	rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
+	rtc->range_max = RTC_TIMESTAMP_END_2099;
+	rtc->owner = THIS_MODULE;
+
+	return rtc_register_device(rtc);
+}
+
+static struct platform_driver wilco_ec_rtc_driver = {
+	.driver = {
+		.name = "rtc-wilco-ec",
+	},
+	.probe = wilco_ec_rtc_probe,
+};
+
+module_platform_driver(wilco_ec_rtc_driver);
+
+MODULE_ALIAS("platform:rtc-wilco-ec");
+MODULE_AUTHOR("Nick Crews <ncrews@google.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Wilco EC RTC driver");
-- 
2.20.1.321.g9e740568ce-goog


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

* Re: [PATCH v3 5/9] platform/chrome: rtc: Add RTC driver
  2019-01-19  0:14 ` [PATCH v3 5/9] platform/chrome: rtc: Add RTC driver Nick Crews
@ 2019-01-22 14:55   ` Enric Balletbo Serra
  0 siblings, 0 replies; 4+ messages in thread
From: Enric Balletbo Serra @ 2019-01-22 14:55 UTC (permalink / raw)
  To: Nick Crews
  Cc: linux-kernel, Guenter Roeck, Simon Glass, Daniel Kurtz, dlaurie,
	Duncan Laurie, linux-rtc, Enric Balletbo i Serra,
	Alessandro Zummo, Benson Leung, Nick Crews, Alexandre Belloni

Hi Nick,

Missatge de Nick Crews <ncrews@chromium.org> del dia ds., 19 de gen.
2019 a les 1:17:
>
> From: Duncan Laurie <dlaurie@google.com>
>
> This Embedded Controller has an internal RTC that is exposed
> as a standard RTC class driver with read/write functionality.
>
> The driver is added to the drivers/rtc/ so that the maintainer of that
> directory will be able to comment on this change, as that maintainer is
> the expert on this system. In addition, the driver code is called
> indirectly after a corresponding device is registered from core.c,
> as opposed to core.c registering the driver callbacks directly.
>
> > hwclock --show --rtc /dev/rtc1
> 2007-12-31 16:01:20.460959-08:00
> > hwclock --systohc --rtc /dev/rtc1
> > hwclock --show --rtc /dev/rtc1
> 2018-11-29 17:08:00.780793-08:00
>
> Signed-off-by: Duncan Laurie <dlaurie@google.com>
> Signed-off-by: Nick Crews <ncrews@chromium.org>
> ---
>
> Changes in v3:
> - rm #define for driver name
> - Don't compute weekday when reading from RTC.
>   Still set weekday when writing, as RTC needs this
>   to control advanced power scheduling
> - rm check for invalid month data
> - Set range_min and range_max on rtc_device
>
> Changes in v2:
> - rm license boiler plate
> - rm "wilco_ec_rtc -" prefix in docstring
> - Make rtc driver its own module within the drivers/rtc/ directory
> - Register a rtc device from core.c that is picked up by this driver
>
>  drivers/platform/chrome/wilco_ec/core.c |  17 +++

Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

>  drivers/rtc/Kconfig                     |  11 ++
>  drivers/rtc/Makefile                    |   1 +
>  drivers/rtc/rtc-wilco-ec.c              | 177 ++++++++++++++++++++++++
>  4 files changed, 206 insertions(+)
>  create mode 100644 drivers/rtc/rtc-wilco-ec.c
>
> diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
> index 9b4a25e63b56..dd8b896b3833 100644
> --- a/drivers/platform/chrome/wilco_ec/core.c
> +++ b/drivers/platform/chrome/wilco_ec/core.c
> @@ -34,6 +34,7 @@ static int wilco_ec_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
>         struct wilco_ec_device *ec;
> +       struct platform_device *child_pdev;
>         int ret;
>
>         ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
> @@ -76,8 +77,24 @@ static int wilco_ec_probe(struct platform_device *pdev)
>                 goto rm_debugfs;
>         }
>
> +       /*
> +        * Register a RTC platform device that will get picked up by the RTC
> +        * subsystem. This platform device will get freed when its parent device
> +        * dev is unregistered.
> +        */
> +       child_pdev = platform_device_register_data(dev, "rtc-wilco-ec",
> +                                                  PLATFORM_DEVID_AUTO,
> +                                                  NULL, 0);
> +       if (IS_ERR(child_pdev)) {
> +               dev_err(dev, "Failed to create RTC platform device\n");
> +               ret = PTR_ERR(child_pdev);
> +               goto remove_sysfs;
> +       }
> +
>         return 0;
>
> +remove_sysfs:
> +       wilco_ec_sysfs_remove(ec);
>  rm_debugfs:
>         wilco_ec_debugfs_remove(ec);
>  destroy_mec:
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 225b0b8516f3..d5063c791515 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1814,4 +1814,15 @@ config RTC_DRV_GOLDFISH
>           Goldfish is a code name for the virtual platform developed by Google
>           for Android emulation.
>
> +config RTC_DRV_WILCO_EC
> +       tristate "Wilco EC RTC"
> +       depends on WILCO_EC
> +       default m
> +       help
> +         If you say yes here, you get read/write support for the Real Time
> +         Clock on the Wilco Embedded Controller (Wilco is a kind of Chromebook)
> +
> +         This can also be built as a module. If so, the module will
> +         be named "rtc_wilco_ec".
> +
>  endif # RTC_CLASS
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index df022d820bee..6255ea78da25 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -172,6 +172,7 @@ obj-$(CONFIG_RTC_DRV_V3020) += rtc-v3020.o
>  obj-$(CONFIG_RTC_DRV_VR41XX)   += rtc-vr41xx.o
>  obj-$(CONFIG_RTC_DRV_VRTC)     += rtc-mrst.o
>  obj-$(CONFIG_RTC_DRV_VT8500)   += rtc-vt8500.o
> +obj-$(CONFIG_RTC_DRV_WILCO_EC) += rtc-wilco-ec.o
>  obj-$(CONFIG_RTC_DRV_WM831X)   += rtc-wm831x.o
>  obj-$(CONFIG_RTC_DRV_WM8350)   += rtc-wm8350.o
>  obj-$(CONFIG_RTC_DRV_X1205)    += rtc-x1205.o
> diff --git a/drivers/rtc/rtc-wilco-ec.c b/drivers/rtc/rtc-wilco-ec.c
> new file mode 100644
> index 000000000000..b721cc3f17f0
> --- /dev/null
> +++ b/drivers/rtc/rtc-wilco-ec.c
> @@ -0,0 +1,177 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RTC interface for Wilco Embedded Controller with R/W abilities
> + *
> + * Copyright 2018 Google LLC
> + *
> + * The corresponding platform device is typically registered in
> + * drivers/platform/chrome/wilco_ec/core.c
> + */
> +
> +#include <linux/bcd.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/wilco-ec.h>
> +#include <linux/rtc.h>
> +#include <linux/timekeeping.h>
> +
> +#define EC_COMMAND_CMOS                        0x7c
> +#define EC_CMOS_TOD_WRITE              0x02
> +#define EC_CMOS_TOD_READ               0x08
> +
> +/**
> + * struct ec_rtc_read - Format of RTC returned by EC.
> + * @second: Second value (0..59)
> + * @minute: Minute value (0..59)
> + * @hour: Hour value (0..23)
> + * @day: Day value (1..31)
> + * @month: Month value (1..12)
> + * @year: Year value (full year % 100)
> + * @century: Century value (full year / 100)
> + *
> + * All values are presented in binary (not BCD).
> + */
> +struct ec_rtc_read {
> +       u8 second;
> +       u8 minute;
> +       u8 hour;
> +       u8 day;
> +       u8 month;
> +       u8 year;
> +       u8 century;
> +} __packed;
> +
> +/**
> + * struct ec_rtc_write - Format of RTC sent to the EC.
> + * @param: EC_CMOS_TOD_WRITE
> + * @century: Century value (full year / 100)
> + * @year: Year value (full year % 100)
> + * @month: Month value (1..12)
> + * @day: Day value (1..31)
> + * @hour: Hour value (0..23)
> + * @minute: Minute value (0..59)
> + * @second: Second value (0..59)
> + * @weekday: Day of the week (0=Saturday)
> + *
> + * All values are presented in BCD.
> + */
> +struct ec_rtc_write {
> +       u8 param;
> +       u8 century;
> +       u8 year;
> +       u8 month;
> +       u8 day;
> +       u8 hour;
> +       u8 minute;
> +       u8 second;
> +       u8 weekday;
> +} __packed;
> +
> +int wilco_ec_rtc_read(struct device *dev, struct rtc_time *tm)
> +{
> +       struct wilco_ec_device *ec = dev_get_drvdata(dev->parent);
> +       u8 param = EC_CMOS_TOD_READ;
> +       struct ec_rtc_read rtc;
> +       struct wilco_ec_message msg = {
> +               .type = WILCO_EC_MSG_LEGACY,
> +               .flags = WILCO_EC_FLAG_RAW_RESPONSE,
> +               .command = EC_COMMAND_CMOS,
> +               .request_data = &param,
> +               .request_size = sizeof(param),
> +               .response_data = &rtc,
> +               .response_size = sizeof(rtc),
> +       };
> +       int ret;
> +
> +       ret = wilco_ec_mailbox(ec, &msg);
> +       if (ret < 0)
> +               return ret;
> +
> +       tm->tm_sec      = rtc.second;
> +       tm->tm_min      = rtc.minute;
> +       tm->tm_hour     = rtc.hour;
> +       tm->tm_mday     = rtc.day;
> +       tm->tm_mon      = rtc.month - 1;
> +       tm->tm_year     = rtc.year + (rtc.century * 100) - 1900;
> +       tm->tm_yday     = rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year);
> +
> +       /* Don't compute day of week, we don't need it. */
> +       tm->tm_wday = -1;
> +
> +       return 0;
> +}
> +
> +int wilco_ec_rtc_write(struct device *dev, struct rtc_time *tm)
> +{
> +       struct wilco_ec_device *ec = dev_get_drvdata(dev->parent);
> +       struct ec_rtc_write rtc;
> +       struct wilco_ec_message msg = {
> +               .type = WILCO_EC_MSG_LEGACY,
> +               .flags = WILCO_EC_FLAG_RAW_RESPONSE,
> +               .command = EC_COMMAND_CMOS,
> +               .request_data = &rtc,
> +               .request_size = sizeof(rtc),
> +       };
> +       int year = tm->tm_year + 1900;
> +       /*
> +        * Convert from 0=Sunday to 0=Saturday for the EC
> +        * We DO need to set weekday because the EC controls battery charging
> +        * schedules that depend on the day of the week.
> +        */
> +       int wday = tm->tm_wday == 6 ? 0 : tm->tm_wday + 1;
> +       int ret;
> +
> +       rtc.param       = EC_CMOS_TOD_WRITE;
> +       rtc.century     = bin2bcd(year / 100);
> +       rtc.year        = bin2bcd(year % 100);
> +       rtc.month       = bin2bcd(tm->tm_mon + 1);
> +       rtc.day         = bin2bcd(tm->tm_mday);
> +       rtc.hour        = bin2bcd(tm->tm_hour);
> +       rtc.minute      = bin2bcd(tm->tm_min);
> +       rtc.second      = bin2bcd(tm->tm_sec);
> +       rtc.weekday     = bin2bcd(wday);
> +
> +       ret = wilco_ec_mailbox(ec, &msg);
> +       if (ret < 0)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +static const struct rtc_class_ops wilco_ec_rtc_ops = {
> +       .read_time = wilco_ec_rtc_read,
> +       .set_time = wilco_ec_rtc_write,
> +};
> +
> +static int wilco_ec_rtc_probe(struct platform_device *pdev)
> +{
> +       struct rtc_device *rtc;
> +
> +       rtc = devm_rtc_allocate_device(&pdev->dev);
> +       if (IS_ERR(rtc))
> +               return PTR_ERR(rtc);
> +
> +       rtc->ops = &wilco_ec_rtc_ops;
> +       /* EC only supports this century */
> +       rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
> +       rtc->range_max = RTC_TIMESTAMP_END_2099;
> +       rtc->owner = THIS_MODULE;
> +
> +       return rtc_register_device(rtc);
> +}
> +
> +static struct platform_driver wilco_ec_rtc_driver = {
> +       .driver = {
> +               .name = "rtc-wilco-ec",
> +       },
> +       .probe = wilco_ec_rtc_probe,
> +};
> +
> +module_platform_driver(wilco_ec_rtc_driver);
> +
> +MODULE_ALIAS("platform:rtc-wilco-ec");
> +MODULE_AUTHOR("Nick Crews <ncrews@google.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Wilco EC RTC driver");
> --
> 2.20.1.321.g9e740568ce-goog
>

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

* Re: [PATCH v3 0/9] platform/chrome: rtc: Add support for Wilco EC
  2019-01-19  0:14 [PATCH v3 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
  2019-01-19  0:14 ` [PATCH v3 5/9] platform/chrome: rtc: Add RTC driver Nick Crews
@ 2019-01-22 15:26 ` Enric Balletbo Serra
  1 sibling, 0 replies; 4+ messages in thread
From: Enric Balletbo Serra @ 2019-01-22 15:26 UTC (permalink / raw)
  To: Nick Crews
  Cc: linux-kernel, Guenter Roeck, Simon Glass, Daniel Kurtz, dlaurie,
	linux-rtc, Enric Balletbo i Serra, Alessandro Zummo,
	Benson Leung, Nick Crews, Duncan Laurie, Alexandre Belloni

Hi Nick,

Missatge de Nick Crews <ncrews@chromium.org> del dia ds., 19 de gen.
2019 a les 1:17:
>
>
> There is a new chromebook that contains a different Embedded Controller
> (codename Wilco) than the rest of the chromebook series. Thus the kernel
> requires a different driver than the already existing and generalized
> cros_ec_* drivers. Specifically, this driver adds support for getting
> and setting the RTC on the EC, adding a binary sysfs attribute
> that receives ACPI events from the EC, adding a binary sysfs
> attribute to request telemetry data from the EC (useful for enterprise
> applications), adding a debugfs interface for sending/receiving raw byte
> sequesnces to the EC, and adding normal sysfs attributes to get/set various
> other properties on the EC. The core of the communication with the EC
> is implemented in wilco_ec/mailbox.c, using a simple byte-level protocol
> with a checksum, transmitted over an eSPI bus. For debugging purposes,
> a raw attribute is also provided which can write/read arbitrary
> bytes to/from the eSPI bus.
>
> We attempted to adhere to the sysfs principles of "one piece of data per
> attribute" as much as possible, and mostly succeded. However, with the
> wilco_ec/adv_power.h attributes, which deal with scheduling power usage,
> we found it most elegant to bundle setting event times for an entire day
> into a single attribute, so at most you are using attributes formatted
> as "%d %d %d %d %d %d". With the telemetry attribute, we had to use a
> binary attribute, instead of the preferable human-readable ascii, in
> order to keep secure the information which is proprietary to the
> enterprise service provider. This opaque binary data will be read and
> sent using a proprietary daemon running on the OS.
>

I reviewed some patches of these series and I think that some of them
are already close to be accepted, however, I still have several doubts
regarding the bunch of sysfs attributes/properties. It is clear for me
that some of them (like the version ones) fit into the sysfs, but I
have several doubts with the power and telemetry attributes, looks to
me that we're abusing of the sysfs interface. I am wondering if
wouldn't be better use a simple ioctl functions that reads and writes
to a chardev (/dev/wilco_ec). Did you think on that possibility? What
people think?

Thanks,
 Enric

> The RTC driver is exposed as a standard RTC driver with
> read/write functionality.
>
> For event notification, the Wilco EC can return extended events that
> are not handled by standard ACPI objects. These events can
> include hotkeys which map to standard functions like brightness
> controls, or information about EC controlled features like the
> charger or battery. These events are triggered with an
> ACPI Notify(0x90) and the event data buffer is read through an ACPI
> method provided by the BIOS which reads the event buffer from EC RAM.
> These events are then processed, with hotkey events being sent
> to the input subsystem and other events put into a queue which
> can be read by a userspace daemon via a sysfs attribute.
>
> The rest of the attributes are categorized as either "properties" or
> "legacy". "legacy" implies that the attribute existed on the EC before it
> was modified for ChromeOS, and "properties" implies that the attribute
> exposes functionality that was added to the EC specifically for
> ChromeOS. They are mostly boolean flags or percentages.
>
> A full thread of the development of these patches can be found at
> https://chromium-review.googlesource.com/c/1371034. This thread contains
> comments and revisions that could be helpful in understanding how the
> driver arrived at the state it is in now. The thread also contains some
> ChromeOS specific patches that actually enable the driver. If you want
> to test the patch yourself, you would have to install the ChromeOS SDK
> and cherry pick in these patches.
>
> I also wrote some integration tests using the Tast testing framework that
> ChromeOS uses. It would require a full ChromeOS SDK to actually run the
> tests, but the source of the tests, written in Go, are useful for
> understanding what the desired behavior is. You can view the tests here:
> https://chromium-review.googlesource.com/c/1372575
>
> Thank you for your comments!
>
> Changes in v3:
> - Change <= to >= in mec_in_range()
> - Add " - EC_HOST_CMD_REGION0" to offset arg for io_bytes_mec()
> - remove unused ret in probe()
> - Add newline spacer in probe()
> - rm unnecessary res in get_resource()
> - s/8bit/8-bit
> - rm first sleep when sending command to EC
> - Move the attribute to the debugfs system
> - Move the implementation to debugfs.c
> - Improve the raw hex parsing
> - Encapsulate global variables in one object
> - Add safety check when passing less than 3 bytes
> - Move documentation to debugfs-wilco-ec
> - explicitly define toplevel_groups from the start,
> so adding telem later makes sense
> - Break version attribute into individual attributes
> - rm unused WILCO_EC_ATTR_RW macro
> - Moved some #defines from legacy.h to legacy.c
> - rm #define for driver name
> - Don't compute weekday when reading from RTC.
>   Still set weekday when writing, as RTC needs this
>   to control advanced power scheduling
> - rm check for invalid month data
> - Set range_min and range_max on rtc_device
> - change err check from "if (ret < 0)" to "if (ret)"
> - Now bubble up error codes from within sysfs_init()
> - Add comment that PID means Property ID
> - rm some useless references to internal docs from documentation
> - add err check on returned data size
> - add check on read request offset and size
>
> Changes in v2:
> - Fixed kernel-doc comments
> - Fixed include of linux/mfd/cros_ec_lpc_mec.h
> - cros_ec_lpc_mec_in_range() returns -EINVAL on error
> - Added parens around macro variables
> - Remove COMPILE_TEST from Kconfig because inb()/outb()
> won't work on anything but X86
> - Add myself as module author
> - Tweak mailbox()
> - Add sysfs documentation
> - rm duplicate EC_MAILBOX_DATA_SIZE defs
> - Make docstrings follow kernel style
> - Fix tags in commit msg
> - Move Kconfig to subdirectory
> - Reading raw now includes ASCII translation
> - Remove license boiler plate
> - Remove "wilco_ec_sysfs -" docstring prefix
> - Fix accidental Makefile deletion
> - Add documentation for sysfs entries
> - Change "enable ? 0 : 1" to "!enable"
> - No longer override error code from sysfs_init()
> - Put attributes in the legacy file to begin with, don't move later
> - Remove duplicate error messages when init()ing sysfs
> - rm license boiler plate
> - rm "wilco_ec_rtc -" prefix in docstring
> - Make rtc driver its own module within the drivers/rtc/ directory
> - Register a rtc device from core.c that is picked up by this driver
> - rm "wilco_ec_event -" prefix from docstring
> - rm license boiler plate
> - Add sysfs directory documentation
> - Fix cosmetics
> - events are init()ed before subdrivers now
> - rm license boiler plate
> - rm "wilco_ec_properties -" prefix from docstring
> - Add documentation
> - rm license boiler plate
> - rm "wilco_ec_adv_power - " prefix from docstring
> - Add documentation
> - make format strings in read() and store() functions static
>
> Duncan Laurie (6):
>   platform/chrome: Remove cros_ec dependency in lpc_mec
>   platform/chrome: Add new driver for Wilco EC
>   platform/chrome: Add support for raw commands in debugfs
>   platform/chrome: Add sysfs attributes
>   platform/chrome: rtc: Add RTC driver
>   platform/chrome: Add event handling
>
> Nick Crews (3):
>   platform/chrome: Add EC properties
>   platform/chrome: Add peakshift and adv_batt_charging
>   platform/chrome: Add binary telemetry attributes
>
>  Documentation/ABI/testing/debugfs-wilco-ec    |  23 +
>  .../ABI/testing/sysfs-platform-wilco-ec       | 196 +++++++
>  drivers/platform/chrome/Kconfig               |   4 +-
>  drivers/platform/chrome/Makefile              |   2 +
>  drivers/platform/chrome/cros_ec_lpc_mec.c     |  52 +-
>  drivers/platform/chrome/cros_ec_lpc_mec.h     |  43 +-
>  drivers/platform/chrome/cros_ec_lpc_reg.c     |  47 +-
>  drivers/platform/chrome/wilco_ec/Kconfig      |  33 ++
>  drivers/platform/chrome/wilco_ec/Makefile     |   6 +
>  drivers/platform/chrome/wilco_ec/adv_power.c  | 544 ++++++++++++++++++
>  drivers/platform/chrome/wilco_ec/adv_power.h  | 183 ++++++
>  drivers/platform/chrome/wilco_ec/core.c       | 154 +++++
>  drivers/platform/chrome/wilco_ec/debugfs.c    | 218 +++++++
>  drivers/platform/chrome/wilco_ec/event.c      | 347 +++++++++++
>  drivers/platform/chrome/wilco_ec/legacy.c     | 103 ++++
>  drivers/platform/chrome/wilco_ec/legacy.h     |  79 +++
>  drivers/platform/chrome/wilco_ec/mailbox.c    | 236 ++++++++
>  drivers/platform/chrome/wilco_ec/properties.c | 344 +++++++++++
>  drivers/platform/chrome/wilco_ec/properties.h | 180 ++++++
>  drivers/platform/chrome/wilco_ec/sysfs.c      | 245 ++++++++
>  drivers/platform/chrome/wilco_ec/telemetry.c  |  73 +++
>  drivers/platform/chrome/wilco_ec/telemetry.h  |  41 ++
>  drivers/platform/chrome/wilco_ec/util.h       |  38 ++
>  drivers/rtc/Kconfig                           |  11 +
>  drivers/rtc/Makefile                          |   1 +
>  drivers/rtc/rtc-wilco-ec.c                    | 177 ++++++
>  include/linux/platform_data/wilco-ec.h        | 189 ++++++
>  27 files changed, 3509 insertions(+), 60 deletions(-)
>  create mode 100644 Documentation/ABI/testing/debugfs-wilco-ec
>  create mode 100644 Documentation/ABI/testing/sysfs-platform-wilco-ec
>  create mode 100644 drivers/platform/chrome/wilco_ec/Kconfig
>  create mode 100644 drivers/platform/chrome/wilco_ec/Makefile
>  create mode 100644 drivers/platform/chrome/wilco_ec/adv_power.c
>  create mode 100644 drivers/platform/chrome/wilco_ec/adv_power.h
>  create mode 100644 drivers/platform/chrome/wilco_ec/core.c
>  create mode 100644 drivers/platform/chrome/wilco_ec/debugfs.c
>  create mode 100644 drivers/platform/chrome/wilco_ec/event.c
>  create mode 100644 drivers/platform/chrome/wilco_ec/legacy.c
>  create mode 100644 drivers/platform/chrome/wilco_ec/legacy.h
>  create mode 100644 drivers/platform/chrome/wilco_ec/mailbox.c
>  create mode 100644 drivers/platform/chrome/wilco_ec/properties.c
>  create mode 100644 drivers/platform/chrome/wilco_ec/properties.h
>  create mode 100644 drivers/platform/chrome/wilco_ec/sysfs.c
>  create mode 100644 drivers/platform/chrome/wilco_ec/telemetry.c
>  create mode 100644 drivers/platform/chrome/wilco_ec/telemetry.h
>  create mode 100644 drivers/platform/chrome/wilco_ec/util.h
>  create mode 100644 drivers/rtc/rtc-wilco-ec.c
>  create mode 100644 include/linux/platform_data/wilco-ec.h
>
> --
> 2.20.1.321.g9e740568ce-goog
>

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

end of thread, other threads:[~2019-01-22 15:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-19  0:14 [PATCH v3 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
2019-01-19  0:14 ` [PATCH v3 5/9] platform/chrome: rtc: Add RTC driver Nick Crews
2019-01-22 14:55   ` Enric Balletbo Serra
2019-01-22 15:26 ` [PATCH v3 0/9] platform/chrome: rtc: Add support for Wilco EC Enric Balletbo Serra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).