[PATCH v6 0/4] platform/chrome: Add basic support for Wilco EC

[PATCH v6 0/4] platform/chrome: Add basic support for Wilco EC

[Nick Crews]

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. 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.

In this patch series I only introduce a very barebones
driver with a debugfs interface for sending/receiving raw byte
sequences to the EC, as well as a standard RTC driver with read/write
functionality.  Later, I will hopefully include more specialized
drivers such as a keyboard backlight driver, an EC event notification node,
telemetry data query node, etc.

The entry point of the driver is wilco_ec/core.c,
which is responsible for several tasks:
- Initialize the register interface that is used by wilco_ec_mailbox()
- Create a platform device which is picked up by the debugfs driver
- Create a platform device which is picked up by the RTC driver

A full thread of the development of these patches can be found at
https://chromium-review.googlesource.com/c/1356082. 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, and even then you would need a Sarien
device to actually run it.

Thank you for your comments!

Changes in v6:
- Re-added WILCO_EC_FLAG_EXTENDED_DATA and went back to always
  reading either EC_MAILBOX_DATA_SIZE or EC_MAILBOX_DATA_SIZE_EXTENDED
  bytes, since it turns out the EC always expects you to calculate the
  checksum over all of them.
- Split WILCO_EC_MSG_TELEMETRY into WILCO_EC_MSG_TELEMETRY_SHORT and
  WILCO_EC_MSG_TELEMETRY_LONG, since there will be two different
  commands.
- A couple comment and err message polishes
- s/4.19/5.1/ for kernel version in documentation, since that is
  the version this patch should land in.
- Instead of requiring at least 3 bytes for msg type and command,
  now just require two for msg type. We can skip the command.
- Fixed error checking in probe() so that errors are hidden, without
  causing more errors or unextpected behavior.
- Some comment polishing.
- In the core, actually unregister the debugfs child platform_device
- In the core, actually unregister the RTC child platform_device.

Changes in v5:
- move checking of NO_RESPONSE flag before timeout check,
  so now timeout doesn't always happen when EC isn't supposed
  to respond.
- rm WILCO_EC_FLAG_EXTENDED_DATA, that is already obvious from
  wilco_ec_message.response_size
- core now always continues regardless of debugfs failure
- mv documentation to file header
- Check for OOM
- rm unneeded check if debug_info is allocqated
- rm bogus comment
- add space around "+"
- rm WILCO_EC_FLAG_EXTENDED_DATA, that is already obvious from
  wilco_ec_message.response_size

Changes in v4:
- add #define DRV_NAME
- remove redundant Kconfig nesting
- changed my email to @chromium.org
- Add better explanation of what core.c does
- Change debugfs driver to be a separate module
- Change me email to @chromium.org from @google.com
- Change CONFIG_WILCO_EC_SYSFS_RAW to
  CONFIG_WILCO_EC_DEBUGFS
- Change me email to @chromium.org from @google.com
- Move "Add RTC driver" before "Add sysfs attributes" so that
  it could get accepted earlier, since it is less contentious

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
- 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:
- 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
- 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

Nick Crews (4):
  cros_ec: 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: rtc: Add RTC driver

 Documentation/ABI/testing/debugfs-wilco-ec |  23 ++
 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   |  21 ++
 drivers/platform/chrome/wilco_ec/Makefile  |   6 +
 drivers/platform/chrome/wilco_ec/core.c    | 136 ++++++++++++
 drivers/platform/chrome/wilco_ec/debugfs.c | 238 +++++++++++++++++++++
 drivers/platform/chrome/wilco_ec/mailbox.c | 236 ++++++++++++++++++++
 drivers/rtc/Kconfig                        |  11 +
 drivers/rtc/Makefile                       |   1 +
 drivers/rtc/rtc-wilco-ec.c                 | 177 +++++++++++++++
 include/linux/platform_data/wilco-ec.h     | 144 +++++++++++++
 15 files changed, 1081 insertions(+), 60 deletions(-)
 create mode 100644 Documentation/ABI/testing/debugfs-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/core.c
 create mode 100644 drivers/platform/chrome/wilco_ec/debugfs.c
 create mode 100644 drivers/platform/chrome/wilco_ec/mailbox.c
 create mode 100644 drivers/rtc/rtc-wilco-ec.c
 create mode 100644 include/linux/platform_data/wilco-ec.h

-- 
2.20.1.611.gfbb209baf1-goog

[PATCH v6 4/4] platform/chrome: rtc: Add RTC driver

[Nick Crews]

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.

To test:
> 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

> 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 v6:
- In the core, actually unregister the RTC child platform_device.

Changes in v5: None
Changes in v4:
- Change me email to @chromium.org from @google.com
- Move "Add RTC driver" before "Add sysfs attributes" so that
  it could get accepted earlier, since it is less contentious

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 |  22 ++-
 drivers/rtc/Kconfig                     |  11 ++
 drivers/rtc/Makefile                    |   1 +
 drivers/rtc/rtc-wilco-ec.c              | 177 ++++++++++++++++++++++++
 include/linux/platform_data/wilco-ec.h  |   2 +
 5 files changed, 211 insertions(+), 2 deletions(-)
 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 ae86cae216fd..e67385ec5103 100644
--- a/drivers/platform/chrome/wilco_ec/core.c
+++ b/drivers/platform/chrome/wilco_ec/core.c
@@ -42,6 +42,7 @@ static int wilco_ec_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct wilco_ec_device *ec;
+	int ret;
 
 	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
 	if (!ec)
@@ -70,21 +71,38 @@ static int wilco_ec_probe(struct platform_device *pdev)
 			     ec->io_packet->start + EC_MAILBOX_DATA_SIZE);
 
 	/*
-	 * Register a debugfs platform device that will get picked up by the
-	 * debugfs driver. Ignore failure.
+	 * Register a child device that will be found by the RTC driver.
+	 * Ignore failure.
 	 */
 	ec->debugfs_pdev = platform_device_register_data(dev,
 							 "wilco-ec-debugfs",
 							 PLATFORM_DEVID_AUTO,
 							 NULL, 0);
 
+	/* Register a child device that will be found by the RTC driver. */
+	ec->rtc_pdev = platform_device_register_data(dev, "rtc-wilco-ec",
+						     PLATFORM_DEVID_AUTO,
+						     NULL, 0);
+	if (IS_ERR(ec->rtc_pdev)) {
+		dev_err(dev, "Failed to create RTC platform device\n");
+		ret = PTR_ERR(ec->rtc_pdev);
+		goto unregister_debugfs;
+	}
+
 	return 0;
+
+unregister_debugfs:
+	if (ec->debugfs_pdev)
+		platform_device_unregister(ec->debugfs_pdev);
+	cros_ec_lpc_mec_destroy();
+	return ret;
 }
 
 static int wilco_ec_remove(struct platform_device *pdev)
 {
 	struct wilco_ec_device *ec = platform_get_drvdata(pdev);
 
+	platform_device_unregister(ec->rtc_pdev);
 	if (ec->debugfs_pdev)
 		platform_device_unregister(ec->debugfs_pdev);
 
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..35cc56114c1c
--- /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@chromium.org>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Wilco EC RTC driver");
diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
index 5344975afa1a..446473a46b88 100644
--- a/include/linux/platform_data/wilco-ec.h
+++ b/include/linux/platform_data/wilco-ec.h
@@ -35,6 +35,7 @@
  *               is used to hold the request and the response.
  * @data_size: Size of the data buffer used for EC communication.
  * @debugfs_pdev: The child platform_device used by the debugfs sub-driver.
+ * @rtc_pdev: The child platform_device used by the RTC sub-driver.
  */
 struct wilco_ec_device {
 	struct device *dev;
@@ -45,6 +46,7 @@ struct wilco_ec_device {
 	void *data_buffer;
 	size_t data_size;
 	struct platform_device *debugfs_pdev;
+	struct platform_device *rtc_pdev;
 };
 
 /**
-- 
2.20.1.611.gfbb209baf1-goog

Re: [PATCH v6 4/4] platform/chrome: rtc: Add RTC driver

[Enric Balletbo i Serra]

Hi,

On 8/2/19 2:23, Nick Crews wrote:
> 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.
> 
> To test:
>> 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
> 
>> 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>

I think that you missed :)

Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

> ---
> 
> Changes in v6:
> - In the core, actually unregister the RTC child platform_device.
> 
> Changes in v5: None
> Changes in v4:
> - Change me email to @chromium.org from @google.com
> - Move "Add RTC driver" before "Add sysfs attributes" so that
>   it could get accepted earlier, since it is less contentious
> 
> 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 |  22 ++-

Alexandre, this specific patch depends on the patches of the series to apply,
and the first patches of the series also conflicts/depends on some patches
queued in chrome-platform-5.1 branch [1]. Is it fine with you if I create an
immutable branch and all go via your tree? Or what do you think? I am new here
and I am not sure how to manage that.

Thanks,
  Enric

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=chrome-platform-5.1

>  drivers/rtc/Kconfig                     |  11 ++
>  drivers/rtc/Makefile                    |   1 +
>  drivers/rtc/rtc-wilco-ec.c              | 177 ++++++++++++++++++++++++
>  include/linux/platform_data/wilco-ec.h  |   2 +
>  5 files changed, 211 insertions(+), 2 deletions(-)
>  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 ae86cae216fd..e67385ec5103 100644
> --- a/drivers/platform/chrome/wilco_ec/core.c
> +++ b/drivers/platform/chrome/wilco_ec/core.c
> @@ -42,6 +42,7 @@ static int wilco_ec_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
>  	struct wilco_ec_device *ec;
> +	int ret;
>  
>  	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
>  	if (!ec)
> @@ -70,21 +71,38 @@ static int wilco_ec_probe(struct platform_device *pdev)
>  			     ec->io_packet->start + EC_MAILBOX_DATA_SIZE);
>  
>  	/*
> -	 * Register a debugfs platform device that will get picked up by the
> -	 * debugfs driver. Ignore failure.
> +	 * Register a child device that will be found by the RTC driver.
> +	 * Ignore failure.
>  	 */
>  	ec->debugfs_pdev = platform_device_register_data(dev,
>  							 "wilco-ec-debugfs",
>  							 PLATFORM_DEVID_AUTO,
>  							 NULL, 0);
>  
> +	/* Register a child device that will be found by the RTC driver. */
> +	ec->rtc_pdev = platform_device_register_data(dev, "rtc-wilco-ec",
> +						     PLATFORM_DEVID_AUTO,
> +						     NULL, 0);
> +	if (IS_ERR(ec->rtc_pdev)) {
> +		dev_err(dev, "Failed to create RTC platform device\n");
> +		ret = PTR_ERR(ec->rtc_pdev);
> +		goto unregister_debugfs;
> +	}
> +
>  	return 0;
> +
> +unregister_debugfs:
> +	if (ec->debugfs_pdev)
> +		platform_device_unregister(ec->debugfs_pdev);
> +	cros_ec_lpc_mec_destroy();
> +	return ret;
>  }
>  
>  static int wilco_ec_remove(struct platform_device *pdev)
>  {
>  	struct wilco_ec_device *ec = platform_get_drvdata(pdev);
>  
> +	platform_device_unregister(ec->rtc_pdev);
>  	if (ec->debugfs_pdev)
>  		platform_device_unregister(ec->debugfs_pdev);
>  
> 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..35cc56114c1c
> --- /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@chromium.org>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Wilco EC RTC driver");
> diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
> index 5344975afa1a..446473a46b88 100644
> --- a/include/linux/platform_data/wilco-ec.h
> +++ b/include/linux/platform_data/wilco-ec.h
> @@ -35,6 +35,7 @@
>   *               is used to hold the request and the response.
>   * @data_size: Size of the data buffer used for EC communication.
>   * @debugfs_pdev: The child platform_device used by the debugfs sub-driver.
> + * @rtc_pdev: The child platform_device used by the RTC sub-driver.
>   */
>  struct wilco_ec_device {
>  	struct device *dev;
> @@ -45,6 +46,7 @@ struct wilco_ec_device {
>  	void *data_buffer;
>  	size_t data_size;
>  	struct platform_device *debugfs_pdev;
> +	struct platform_device *rtc_pdev;
>  };
>  
>  /**
> 

Re: [PATCH v6 4/4] platform/chrome: rtc: Add RTC driver

[Nick Crews]

Hi Enric and Alexandre,

On Fri, Feb 8, 2019 at 5:18 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi,
>
> On 8/2/19 2:23, Nick Crews wrote:
> > 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.
> >
> > To test:
> >> 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
> >
> >> 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>
>
> I think that you missed :)
>
> Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
>

Ah, I removed these because this version is a bit different from the
one that you Acked. Do you normally keep those sign-offs
even after making changes that you possibly might not like?

> > ---
> >
> > Changes in v6:
> > - In the core, actually unregister the RTC child platform_device.
> >
> > Changes in v5: None
> > Changes in v4:
> > - Change me email to @chromium.org from @google.com
> > - Move "Add RTC driver" before "Add sysfs attributes" so that
> >   it could get accepted earlier, since it is less contentious
> >
> > 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 |  22 ++-
>
> Alexandre, this specific patch depends on the patches of the series to apply,
> and the first patches of the series also conflicts/depends on some patches
> queued in chrome-platform-5.1 branch [1]. Is it fine with you if I create an
> immutable branch and all go via your tree? Or what do you think? I am new here
> and I am not sure how to manage that.
>
> Thanks,
>   Enric

Let me know if I can help with this. I based this off what I thought was the
most current master, SHA 74e96711e3379fc66630f2a1d184947f80cf2c48.
Was this not the correct commit to base these off of?

>
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=chrome-platform-5.1
>
> >  drivers/rtc/Kconfig                     |  11 ++
> >  drivers/rtc/Makefile                    |   1 +
> >  drivers/rtc/rtc-wilco-ec.c              | 177 ++++++++++++++++++++++++
> >  include/linux/platform_data/wilco-ec.h  |   2 +
> >  5 files changed, 211 insertions(+), 2 deletions(-)
> >  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 ae86cae216fd..e67385ec5103 100644
> > --- a/drivers/platform/chrome/wilco_ec/core.c
> > +++ b/drivers/platform/chrome/wilco_ec/core.c
> > @@ -42,6 +42,7 @@ static int wilco_ec_probe(struct platform_device *pdev)
> >  {
> >       struct device *dev = &pdev->dev;
> >       struct wilco_ec_device *ec;
> > +     int ret;
> >
> >       ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
> >       if (!ec)
> > @@ -70,21 +71,38 @@ static int wilco_ec_probe(struct platform_device *pdev)
> >                            ec->io_packet->start + EC_MAILBOX_DATA_SIZE);
> >
> >       /*
> > -      * Register a debugfs platform device that will get picked up by the
> > -      * debugfs driver. Ignore failure.
> > +      * Register a child device that will be found by the RTC driver.
> > +      * Ignore failure.
> >        */
> >       ec->debugfs_pdev = platform_device_register_data(dev,
> >                                                        "wilco-ec-debugfs",
> >                                                        PLATFORM_DEVID_AUTO,
> >                                                        NULL, 0);
> >
> > +     /* Register a child device that will be found by the RTC driver. */
> > +     ec->rtc_pdev = platform_device_register_data(dev, "rtc-wilco-ec",
> > +                                                  PLATFORM_DEVID_AUTO,
> > +                                                  NULL, 0);
> > +     if (IS_ERR(ec->rtc_pdev)) {
> > +             dev_err(dev, "Failed to create RTC platform device\n");
> > +             ret = PTR_ERR(ec->rtc_pdev);
> > +             goto unregister_debugfs;
> > +     }
> > +
> >       return 0;
> > +
> > +unregister_debugfs:
> > +     if (ec->debugfs_pdev)
> > +             platform_device_unregister(ec->debugfs_pdev);
> > +     cros_ec_lpc_mec_destroy();
> > +     return ret;
> >  }
> >
> >  static int wilco_ec_remove(struct platform_device *pdev)
> >  {
> >       struct wilco_ec_device *ec = platform_get_drvdata(pdev);
> >
> > +     platform_device_unregister(ec->rtc_pdev);
> >       if (ec->debugfs_pdev)
> >               platform_device_unregister(ec->debugfs_pdev);
> >
> > 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..35cc56114c1c
> > --- /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@chromium.org>");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("Wilco EC RTC driver");
> > diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
> > index 5344975afa1a..446473a46b88 100644
> > --- a/include/linux/platform_data/wilco-ec.h
> > +++ b/include/linux/platform_data/wilco-ec.h
> > @@ -35,6 +35,7 @@
> >   *               is used to hold the request and the response.
> >   * @data_size: Size of the data buffer used for EC communication.
> >   * @debugfs_pdev: The child platform_device used by the debugfs sub-driver.
> > + * @rtc_pdev: The child platform_device used by the RTC sub-driver.
> >   */
> >  struct wilco_ec_device {
> >       struct device *dev;
> > @@ -45,6 +46,7 @@ struct wilco_ec_device {
> >       void *data_buffer;
> >       size_t data_size;
> >       struct platform_device *debugfs_pdev;
> > +     struct platform_device *rtc_pdev;
> >  };
> >
> >  /**
> >

Re: [PATCH v6 4/4] platform/chrome: rtc: Add RTC driver

[Alexandre Belloni]

On 08/02/2019 13:18:38+0100, Enric Balletbo i Serra wrote:
> > Changes in v6:
> > - In the core, actually unregister the RTC child platform_device.
> > 
> > Changes in v5: None
> > Changes in v4:
> > - Change me email to @chromium.org from @google.com
> > - Move "Add RTC driver" before "Add sysfs attributes" so that
> >   it could get accepted earlier, since it is less contentious
> > 
> > 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 |  22 ++-
> 
> Alexandre, this specific patch depends on the patches of the series to apply,
> and the first patches of the series also conflicts/depends on some patches
> queued in chrome-platform-5.1 branch [1]. Is it fine with you if I create an
> immutable branch and all go via your tree? Or what do you think? I am new here
> and I am not sure how to manage that.
> 

The series should probably go through the chrome tree. The only possible
conflict would be in drivers/rtc/Makefile and it should be easy enough
for Linus to solve.

> Thanks,
>   Enric
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=chrome-platform-5.1
> 
> >  drivers/rtc/Kconfig                     |  11 ++
> >  drivers/rtc/Makefile                    |   1 +
> >  drivers/rtc/rtc-wilco-ec.c              | 177 ++++++++++++++++++++++++
> >  include/linux/platform_data/wilco-ec.h  |   2 +
> >  5 files changed, 211 insertions(+), 2 deletions(-)
> >  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 ae86cae216fd..e67385ec5103 100644
> > --- a/drivers/platform/chrome/wilco_ec/core.c
> > +++ b/drivers/platform/chrome/wilco_ec/core.c
> > @@ -42,6 +42,7 @@ static int wilco_ec_probe(struct platform_device *pdev)
> >  {
> >  	struct device *dev = &pdev->dev;
> >  	struct wilco_ec_device *ec;
> > +	int ret;
> >  
> >  	ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
> >  	if (!ec)
> > @@ -70,21 +71,38 @@ static int wilco_ec_probe(struct platform_device *pdev)
> >  			     ec->io_packet->start + EC_MAILBOX_DATA_SIZE);
> >  
> >  	/*
> > -	 * Register a debugfs platform device that will get picked up by the
> > -	 * debugfs driver. Ignore failure.
> > +	 * Register a child device that will be found by the RTC driver.
> > +	 * Ignore failure.
> >  	 */
> >  	ec->debugfs_pdev = platform_device_register_data(dev,
> >  							 "wilco-ec-debugfs",
> >  							 PLATFORM_DEVID_AUTO,
> >  							 NULL, 0);
> >  
> > +	/* Register a child device that will be found by the RTC driver. */
> > +	ec->rtc_pdev = platform_device_register_data(dev, "rtc-wilco-ec",
> > +						     PLATFORM_DEVID_AUTO,
> > +						     NULL, 0);
> > +	if (IS_ERR(ec->rtc_pdev)) {
> > +		dev_err(dev, "Failed to create RTC platform device\n");
> > +		ret = PTR_ERR(ec->rtc_pdev);
> > +		goto unregister_debugfs;
> > +	}
> > +
> >  	return 0;
> > +
> > +unregister_debugfs:
> > +	if (ec->debugfs_pdev)
> > +		platform_device_unregister(ec->debugfs_pdev);
> > +	cros_ec_lpc_mec_destroy();
> > +	return ret;
> >  }
> >  
> >  static int wilco_ec_remove(struct platform_device *pdev)
> >  {
> >  	struct wilco_ec_device *ec = platform_get_drvdata(pdev);
> >  
> > +	platform_device_unregister(ec->rtc_pdev);
> >  	if (ec->debugfs_pdev)
> >  		platform_device_unregister(ec->debugfs_pdev);
> >  
> > 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..35cc56114c1c
> > --- /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@chromium.org>");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("Wilco EC RTC driver");
> > diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
> > index 5344975afa1a..446473a46b88 100644
> > --- a/include/linux/platform_data/wilco-ec.h
> > +++ b/include/linux/platform_data/wilco-ec.h
> > @@ -35,6 +35,7 @@
> >   *               is used to hold the request and the response.
> >   * @data_size: Size of the data buffer used for EC communication.
> >   * @debugfs_pdev: The child platform_device used by the debugfs sub-driver.
> > + * @rtc_pdev: The child platform_device used by the RTC sub-driver.
> >   */
> >  struct wilco_ec_device {
> >  	struct device *dev;
> > @@ -45,6 +46,7 @@ struct wilco_ec_device {
> >  	void *data_buffer;
> >  	size_t data_size;
> >  	struct platform_device *debugfs_pdev;
> > +	struct platform_device *rtc_pdev;
> >  };
> >  
> >  /**
> > 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 4/4] platform/chrome: rtc: Add RTC driver

[Alexandre Belloni]

On 08/02/2019 09:10:02-0700, Nick Crews wrote:
> Hi Enric and Alexandre,
> 
> On Fri, Feb 8, 2019 at 5:18 AM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
> >
> > Hi,
> >
> > On 8/2/19 2:23, Nick Crews wrote:
> > > 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.
> > >
> > > To test:
> > >> 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
> > >
> > >> 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>
> >
> > I think that you missed :)
> >
> > Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> >
> 
> Ah, I removed these because this version is a bit different from the
> one that you Acked. Do you normally keep those sign-offs
> even after making changes that you possibly might not like?
> 

I would think you could have kept mine.

> > > ---
> > >
> > > Changes in v6:
> > > - In the core, actually unregister the RTC child platform_device.
> > >
> > > Changes in v5: None
> > > Changes in v4:
> > > - Change me email to @chromium.org from @google.com
> > > - Move "Add RTC driver" before "Add sysfs attributes" so that
> > >   it could get accepted earlier, since it is less contentious
> > >
> > > 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 |  22 ++-
> >
> > Alexandre, this specific patch depends on the patches of the series to apply,
> > and the first patches of the series also conflicts/depends on some patches
> > queued in chrome-platform-5.1 branch [1]. Is it fine with you if I create an
> > immutable branch and all go via your tree? Or what do you think? I am new here
> > and I am not sure how to manage that.
> >
> > Thanks,
> >   Enric
> 
> Let me know if I can help with this. I based this off what I thought was the
> most current master, SHA 74e96711e3379fc66630f2a1d184947f80cf2c48.
> Was this not the correct commit to base these off of?
> 
> >
> > [1]
> > https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git/log/?h=chrome-platform-5.1
> >
> > >  drivers/rtc/Kconfig                     |  11 ++
> > >  drivers/rtc/Makefile                    |   1 +
> > >  drivers/rtc/rtc-wilco-ec.c              | 177 ++++++++++++++++++++++++
> > >  include/linux/platform_data/wilco-ec.h  |   2 +
> > >  5 files changed, 211 insertions(+), 2 deletions(-)
> > >  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 ae86cae216fd..e67385ec5103 100644
> > > --- a/drivers/platform/chrome/wilco_ec/core.c
> > > +++ b/drivers/platform/chrome/wilco_ec/core.c
> > > @@ -42,6 +42,7 @@ static int wilco_ec_probe(struct platform_device *pdev)
> > >  {
> > >       struct device *dev = &pdev->dev;
> > >       struct wilco_ec_device *ec;
> > > +     int ret;
> > >
> > >       ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
> > >       if (!ec)
> > > @@ -70,21 +71,38 @@ static int wilco_ec_probe(struct platform_device *pdev)
> > >                            ec->io_packet->start + EC_MAILBOX_DATA_SIZE);
> > >
> > >       /*
> > > -      * Register a debugfs platform device that will get picked up by the
> > > -      * debugfs driver. Ignore failure.
> > > +      * Register a child device that will be found by the RTC driver.
> > > +      * Ignore failure.
> > >        */
> > >       ec->debugfs_pdev = platform_device_register_data(dev,
> > >                                                        "wilco-ec-debugfs",
> > >                                                        PLATFORM_DEVID_AUTO,
> > >                                                        NULL, 0);
> > >
> > > +     /* Register a child device that will be found by the RTC driver. */
> > > +     ec->rtc_pdev = platform_device_register_data(dev, "rtc-wilco-ec",
> > > +                                                  PLATFORM_DEVID_AUTO,
> > > +                                                  NULL, 0);
> > > +     if (IS_ERR(ec->rtc_pdev)) {
> > > +             dev_err(dev, "Failed to create RTC platform device\n");
> > > +             ret = PTR_ERR(ec->rtc_pdev);
> > > +             goto unregister_debugfs;
> > > +     }
> > > +
> > >       return 0;
> > > +
> > > +unregister_debugfs:
> > > +     if (ec->debugfs_pdev)
> > > +             platform_device_unregister(ec->debugfs_pdev);
> > > +     cros_ec_lpc_mec_destroy();
> > > +     return ret;
> > >  }
> > >
> > >  static int wilco_ec_remove(struct platform_device *pdev)
> > >  {
> > >       struct wilco_ec_device *ec = platform_get_drvdata(pdev);
> > >
> > > +     platform_device_unregister(ec->rtc_pdev);
> > >       if (ec->debugfs_pdev)
> > >               platform_device_unregister(ec->debugfs_pdev);
> > >
> > > 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..35cc56114c1c
> > > --- /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@chromium.org>");
> > > +MODULE_LICENSE("GPL v2");
> > > +MODULE_DESCRIPTION("Wilco EC RTC driver");
> > > diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
> > > index 5344975afa1a..446473a46b88 100644
> > > --- a/include/linux/platform_data/wilco-ec.h
> > > +++ b/include/linux/platform_data/wilco-ec.h
> > > @@ -35,6 +35,7 @@
> > >   *               is used to hold the request and the response.
> > >   * @data_size: Size of the data buffer used for EC communication.
> > >   * @debugfs_pdev: The child platform_device used by the debugfs sub-driver.
> > > + * @rtc_pdev: The child platform_device used by the RTC sub-driver.
> > >   */
> > >  struct wilco_ec_device {
> > >       struct device *dev;
> > > @@ -45,6 +46,7 @@ struct wilco_ec_device {
> > >       void *data_buffer;
> > >       size_t data_size;
> > >       struct platform_device *debugfs_pdev;
> > > +     struct platform_device *rtc_pdev;
> > >  };
> > >
> > >  /**
> > >

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v6 4/4] platform/chrome: rtc: Add RTC driver

[Enric Balletbo Serra]

Hi Alexandre,

[snip]

>
> The series should probably go through the chrome tree. The only possible
> conflict would be in drivers/rtc/Makefile and it should be easy enough
> for Linus to solve.
>

Perfect, I will pick also the RTC patch then.

[snip]

Re: [PATCH v6 4/4] platform/chrome: rtc: Add RTC driver

[Enric Balletbo Serra]

Hi Nick,
Missatge de Nick Crews <ncrews@chromium.org> del dia dv., 8 de febr.
2019 a les 17:10:
>
> Hi Enric and Alexandre,
>
> On Fri, Feb 8, 2019 at 5:18 AM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
> >
> > Hi,
> >
> > On 8/2/19 2:23, Nick Crews wrote:
> > > 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.
> > >
> > > To test:
> > >> 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
> > >
> > >> 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>
> >
> > I think that you missed :)
> >
> > Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
> >
>
> Ah, I removed these because this version is a bit different from the
> one that you Acked. Do you normally keep those sign-offs
> even after making changes that you possibly might not like?
>

Well, it's right that you did a small change in the core, so to be
strict maybe you have reason, but I don't see as a big change so I
expected you maintain my tag. Anyway it's not a problem at all, just
that at least I expected the Alexandre ack :)

[snip]

> >
> > Alexandre, this specific patch depends on the patches of the series to apply,
> > and the first patches of the series also conflicts/depends on some patches
> > queued in chrome-platform-5.1 branch [1]. Is it fine with you if I create an
> > immutable branch and all go via your tree? Or what do you think? I am new here
> > and I am not sure how to manage that.
> >
> > Thanks,
> >   Enric
>
> Let me know if I can help with this. I based this off what I thought was the
> most current master, SHA 74e96711e3379fc66630f2a1d184947f80cf2c48.
> Was this not the correct commit to base these off of?
>

Yeah, it's fine, just that I queued recently some changes and I have
trivial conflicts in Kconfig and Makefile, nothing that you need to
worry.

[snip]