linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/9] platform/chrome: rtc: Add support for Wilco EC
@ 2019-01-14 22:03 Nick Crews
  2019-01-14 22:03 ` [PATCH v2 1/9] platform/chrome: Remove cros_ec dependency in lpc_mec Nick Crews
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Nick Crews @ 2019-01-14 22:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: groeck, sjg, dlaurie, Nick Crews, linux-rtc,
	Enric Balletbo i Serra, Alessandro Zummo, Benson Leung,
	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), 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. Finally, the
"version" attribute returns a formatted result that looks something
like:
> cat /sys/bus/platform/devices/GOOG000C\:00/version
Label        : 95.00.06
SVN Revision : 5960a.06
Model Number : 08;8
Build Date   : 11/29/18

The RTC driver is exposed as a standard RTC class 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 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
- Removed COMPILE_TEST from Kconfig because inb()/outb()
won't work on anything but X86
- Moved everything to wilco_ec/ subdirectory
- Moved header file to include/platform_data/
so could be used by future sub-drivers
- Tweaked mailbox()
- Removed duplicate EC_MAILBOX_DATA_SIZE defs
- Removed some error messages / converted to dbg()
- 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 in sysfs_init() and its caller
- Add sysfs documentation
- rm duplicate EC_MAILBOX_DATA_SIZE defs
- Make docstrings follow kernel style
- Fix tags in commit msg
- 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
- rm unused rtc_sync() function
- rm "wilco_ec_event -" prefix from docstring
- rm license boiler plate
- Add sysfs directory documentation
- Fix cosmetics
- Remove duplicate error messages
- 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
- Made format strings in store() and read() functions static
- rm "wilco_ec_telemetry - " prefix from docstring
- rm license boiler plate
- Fix commit msg tag

Duncan Laurie (6):
  platform/chrome: Remove cros_ec dependency in lpc_mec
  platform/chrome: Add new driver for Wilco EC
  platform/chrome: Add sysfs attributes
  platform/chrome: Add support for raw commands in sysfs
  platform/chrome: rtc: Add RTC driver for Wilco EC
  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

 .../ABI/testing/sysfs-platform-wilcoec        | 176 ++++++
 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     |  43 +-
 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       | 144 +++++
 drivers/platform/chrome/wilco_ec/event.c      | 347 +++++++++++
 drivers/platform/chrome/wilco_ec/legacy.c     | 190 ++++++
 drivers/platform/chrome/wilco_ec/legacy.h     |  93 +++
 drivers/platform/chrome/wilco_ec/mailbox.c    | 237 ++++++++
 drivers/platform/chrome/wilco_ec/properties.c | 344 +++++++++++
 drivers/platform/chrome/wilco_ec/properties.h | 179 ++++++
 drivers/platform/chrome/wilco_ec/sysfs.c      | 238 ++++++++
 drivers/platform/chrome/wilco_ec/telemetry.c  |  66 +++
 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                    | 178 ++++++
 include/linux/platform_data/wilco-ec.h        | 186 ++++++
 25 files changed, 3319 insertions(+), 60 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-wilcoec
 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/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.97.g81188d93c3-goog


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

* [PATCH v2 1/9] platform/chrome: Remove cros_ec dependency in lpc_mec
  2019-01-14 22:03 [PATCH v2 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
@ 2019-01-14 22:03 ` Nick Crews
  2019-01-14 22:03 ` [PATCH v2 2/9] platform/chrome: Add new driver for Wilco EC Nick Crews
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Nick Crews @ 2019-01-14 22:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: groeck, sjg, dlaurie, Duncan Laurie, Nick Crews,
	Enric Balletbo i Serra, Benson Leung

From: Duncan Laurie <dlaurie@google.com>

In order to allow this code to be re-used, remove the dependency
on the rest of the cros_ec code from the cros_ec_lpc_mec functions.

Instead of using a hardcoded register base address of 0x800 have
this be passed in to cros_ec_lpc_mec_init().  The existing cros_ec
use case now passes in the 0x800 base address this way.

There are some error checks that happen in cros_ec_lpc_mec_in_range()
that probably shouldn't be there, as they are checking kernel-space
callers and not user-space input. However, we'll just do the refactor in
this patch, and in a future patch might remove this error checking and
fix all the instances of code that calls this.

There's a similar problem in cros_ec_lpc_read_bytes(), where we return a
checksum, but on error just return 0. This should probably be changed so
that it returns int, but we don't want to have to mess with all the
calling code for this fix. Maybe we'll come back through later and fix
this.

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

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

 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 | 43 ++++++-------------
 3 files changed, 79 insertions(+), 59 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_lpc_mec.c b/drivers/platform/chrome/cros_ec_lpc_mec.c
index c4edfa83e493..36f22aacfeb4 100644
--- a/drivers/platform/chrome/cros_ec_lpc_mec.c
+++ b/drivers/platform/chrome/cros_ec_lpc_mec.c
@@ -23,7 +23,6 @@
 
 #include <linux/delay.h>
 #include <linux/io.h>
-#include <linux/mfd/cros_ec_commands.h>
 #include <linux/mutex.h>
 #include <linux/types.h>
 
@@ -34,6 +33,7 @@
  * EC mutex because memmap data may be accessed without it being held.
  */
 static struct mutex io_mutex;
+static u16 mec_emi_base, mec_emi_end;
 
 /*
  * cros_ec_lpc_mec_emi_write_address
@@ -46,10 +46,37 @@ static struct mutex io_mutex;
 static void cros_ec_lpc_mec_emi_write_address(u16 addr,
 			enum cros_ec_lpc_mec_emi_access_mode access_type)
 {
-	/* Address relative to start of EMI range */
-	addr -= MEC_EMI_RANGE_START;
-	outb((addr & 0xfc) | access_type, MEC_EMI_EC_ADDRESS_B0);
-	outb((addr >> 8) & 0x7f, MEC_EMI_EC_ADDRESS_B1);
+	outb((addr & 0xfc) | access_type, MEC_EMI_EC_ADDRESS_B0(mec_emi_base));
+	outb((addr >> 8) & 0x7f, MEC_EMI_EC_ADDRESS_B1(mec_emi_base));
+}
+
+/**
+ * cros_ec_lpc_mec_in_range() - Determine if addresses are in MEC EMI range.
+ *
+ * @offset: Address offset
+ * @length: Number of bytes to check
+ *
+ * Return: 1 if in range, 0 if not, and -EINVAL on failure
+ *         such as the mec range not being initialized
+ */
+int cros_ec_lpc_mec_in_range(unsigned int offset, unsigned int length)
+{
+	if (length == 0)
+		return -EINVAL;
+
+	if (WARN_ON(mec_emi_base == 0 || mec_emi_end == 0))
+		return -EINVAL;
+
+	if (offset >= mec_emi_base && offset < mec_emi_end) {
+		if (WARN_ON(offset + length - 1 <= mec_emi_end))
+			return -EINVAL;
+		return 1;
+	}
+
+	if (WARN_ON(offset + length > mec_emi_base && offset < mec_emi_end))
+		return -EINVAL;
+
+	return 0;
 }
 
 /*
@@ -71,6 +98,11 @@ u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
 	u8 sum = 0;
 	enum cros_ec_lpc_mec_emi_access_mode access, new_access;
 
+	/* Return checksum of 0 if window is not initialized */
+	WARN_ON(mec_emi_base == 0 || mec_emi_end == 0);
+	if (mec_emi_base == 0 || mec_emi_end == 0)
+		return 0;
+
 	/*
 	 * Long access cannot be used on misaligned data since reading B0 loads
 	 * the data register and writing B3 flushes.
@@ -86,9 +118,9 @@ u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
 	cros_ec_lpc_mec_emi_write_address(offset, access);
 
 	/* Skip bytes in case of misaligned offset */
-	io_addr = MEC_EMI_EC_DATA_B0 + (offset & 0x3);
+	io_addr = MEC_EMI_EC_DATA_B0(mec_emi_base) + (offset & 0x3);
 	while (i < length) {
-		while (io_addr <= MEC_EMI_EC_DATA_B3) {
+		while (io_addr <= MEC_EMI_EC_DATA_B3(mec_emi_base)) {
 			if (io_type == MEC_IO_READ)
 				buf[i] = inb(io_addr++);
 			else
@@ -118,7 +150,7 @@ u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
 		}
 
 		/* Access [B0, B3] on each loop pass */
-		io_addr = MEC_EMI_EC_DATA_B0;
+		io_addr = MEC_EMI_EC_DATA_B0(mec_emi_base);
 	}
 
 done:
@@ -128,9 +160,11 @@ u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
 }
 EXPORT_SYMBOL(cros_ec_lpc_io_bytes_mec);
 
-void cros_ec_lpc_mec_init(void)
+void cros_ec_lpc_mec_init(unsigned int base, unsigned int end)
 {
 	mutex_init(&io_mutex);
+	mec_emi_base = base;
+	mec_emi_end = end;
 }
 EXPORT_SYMBOL(cros_ec_lpc_mec_init);
 
diff --git a/drivers/platform/chrome/cros_ec_lpc_mec.h b/drivers/platform/chrome/cros_ec_lpc_mec.h
index 105068c0e919..896fffd174b3 100644
--- a/drivers/platform/chrome/cros_ec_lpc_mec.h
+++ b/drivers/platform/chrome/cros_ec_lpc_mec.h
@@ -24,8 +24,6 @@
 #ifndef __CROS_EC_LPC_MEC_H
 #define __CROS_EC_LPC_MEC_H
 
-#include <linux/mfd/cros_ec_commands.h>
-
 enum cros_ec_lpc_mec_emi_access_mode {
 	/* 8-bit access */
 	ACCESS_TYPE_BYTE = 0x0,
@@ -45,27 +43,23 @@ enum cros_ec_lpc_mec_io_type {
 	MEC_IO_WRITE,
 };
 
-/* Access IO ranges 0x800 thru 0x9ff using EMI interface instead of LPC */
-#define MEC_EMI_RANGE_START EC_HOST_CMD_REGION0
-#define MEC_EMI_RANGE_END   (EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE)
-
 /* EMI registers are relative to base */
-#define MEC_EMI_BASE 0x800
-#define MEC_EMI_HOST_TO_EC (MEC_EMI_BASE + 0)
-#define MEC_EMI_EC_TO_HOST (MEC_EMI_BASE + 1)
-#define MEC_EMI_EC_ADDRESS_B0 (MEC_EMI_BASE + 2)
-#define MEC_EMI_EC_ADDRESS_B1 (MEC_EMI_BASE + 3)
-#define MEC_EMI_EC_DATA_B0 (MEC_EMI_BASE + 4)
-#define MEC_EMI_EC_DATA_B1 (MEC_EMI_BASE + 5)
-#define MEC_EMI_EC_DATA_B2 (MEC_EMI_BASE + 6)
-#define MEC_EMI_EC_DATA_B3 (MEC_EMI_BASE + 7)
+#define MEC_EMI_HOST_TO_EC(MEC_EMI_BASE)	((MEC_EMI_BASE) + 0)
+#define MEC_EMI_EC_TO_HOST(MEC_EMI_BASE)	((MEC_EMI_BASE) + 1)
+#define MEC_EMI_EC_ADDRESS_B0(MEC_EMI_BASE)	((MEC_EMI_BASE) + 2)
+#define MEC_EMI_EC_ADDRESS_B1(MEC_EMI_BASE)	((MEC_EMI_BASE) + 3)
+#define MEC_EMI_EC_DATA_B0(MEC_EMI_BASE)	((MEC_EMI_BASE) + 4)
+#define MEC_EMI_EC_DATA_B1(MEC_EMI_BASE)	((MEC_EMI_BASE) + 5)
+#define MEC_EMI_EC_DATA_B2(MEC_EMI_BASE)	((MEC_EMI_BASE) + 6)
+#define MEC_EMI_EC_DATA_B3(MEC_EMI_BASE)	((MEC_EMI_BASE) + 7)
 
-/*
- * cros_ec_lpc_mec_init
+/**
+ * cros_ec_lpc_mec_init() - Initialize MEC I/O.
  *
- * Initialize MEC I/O.
+ * @base: MEC EMI Base address
+ * @end: MEC EMI End address
  */
-void cros_ec_lpc_mec_init(void);
+void cros_ec_lpc_mec_init(unsigned int base, unsigned int end);
 
 /*
  * cros_ec_lpc_mec_destroy
@@ -74,6 +68,17 @@ void cros_ec_lpc_mec_init(void);
  */
 void cros_ec_lpc_mec_destroy(void);
 
+/**
+ * cros_ec_lpc_mec_in_range() - Determine if addresses are in MEC EMI range.
+ *
+ * @offset: Address offset
+ * @length: Number of bytes to check
+ *
+ * Return: 1 if in range, 0 if not, and -EINVAL on failure
+ *         such as the mec range not being initialized
+ */
+int cros_ec_lpc_mec_in_range(unsigned int offset, unsigned int length);
+
 /**
  * cros_ec_lpc_io_bytes_mec - Read / write bytes to MEC EMI port
  *
diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c
index fc23d535c404..4cba259b5b1e 100644
--- a/drivers/platform/chrome/cros_ec_lpc_reg.c
+++ b/drivers/platform/chrome/cros_ec_lpc_reg.c
@@ -59,51 +59,32 @@ static u8 lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
 
 u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
 {
-	if (length == 0)
-		return 0;
-
-	/* Access desired range through EMI interface */
-	if (offset >= MEC_EMI_RANGE_START && offset <= MEC_EMI_RANGE_END) {
-		/* Ensure we don't straddle EMI region */
-		if (WARN_ON(offset + length - 1 > MEC_EMI_RANGE_END))
-			return 0;
+	int in_range = cros_ec_lpc_mec_in_range(offset, length);
 
-		return cros_ec_lpc_io_bytes_mec(MEC_IO_READ, offset, length,
-						dest);
-	}
-
-	if (WARN_ON(offset + length > MEC_EMI_RANGE_START &&
-		    offset < MEC_EMI_RANGE_START))
+	if (in_range < 0)
 		return 0;
 
-	return lpc_read_bytes(offset, length, dest);
+	return in_range ?
+		cros_ec_lpc_io_bytes_mec(MEC_IO_READ, offset, length, dest) :
+		lpc_read_bytes(offset, length, dest);
 }
 
 u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
 {
-	if (length == 0)
-		return 0;
-
-	/* Access desired range through EMI interface */
-	if (offset >= MEC_EMI_RANGE_START && offset <= MEC_EMI_RANGE_END) {
-		/* Ensure we don't straddle EMI region */
-		if (WARN_ON(offset + length - 1 > MEC_EMI_RANGE_END))
-			return 0;
+	int in_range = cros_ec_lpc_mec_in_range(offset, length);
 
-		return cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, offset, length,
-						msg);
-	}
-
-	if (WARN_ON(offset + length > MEC_EMI_RANGE_START &&
-		    offset < MEC_EMI_RANGE_START))
+	if (in_range < 0)
 		return 0;
 
-	return lpc_write_bytes(offset, length, msg);
+	return in_range ?
+		cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, offset, length, msg) :
+		lpc_write_bytes(offset, length, msg);
 }
 
 void cros_ec_lpc_reg_init(void)
 {
-	cros_ec_lpc_mec_init();
+	cros_ec_lpc_mec_init(EC_HOST_CMD_REGION0,
+			     EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE);
 }
 
 void cros_ec_lpc_reg_destroy(void)
-- 
2.20.1.97.g81188d93c3-goog


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

* [PATCH v2 2/9] platform/chrome: Add new driver for Wilco EC
  2019-01-14 22:03 [PATCH v2 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
  2019-01-14 22:03 ` [PATCH v2 1/9] platform/chrome: Remove cros_ec dependency in lpc_mec Nick Crews
@ 2019-01-14 22:03 ` Nick Crews
  2019-01-15 18:10   ` Enric Balletbo Serra
  2019-01-14 22:03 ` [PATCH v2 3/9] platform/chrome: Add sysfs attributes Nick Crews
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Nick Crews @ 2019-01-14 22:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: groeck, sjg, dlaurie, Duncan Laurie, Nick Crews,
	Enric Balletbo i Serra, Benson Leung

From: Duncan Laurie <dlaurie@google.com>

This EC is an incompatible variant of the typical Chrome OS embedded
controller.  It uses the same low-level communication and a similar
protocol with some significant differences.  The EC firmware does
not support the same mailbox commands so it is not registered as a
cros_ec device type.

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

Changes in v2:
- Removed COMPILE_TEST from Kconfig because inb()/outb()
won't work on anything but X86
- Moved everything to wilco_ec/ subdirectory
- Moved header file to include/platform_data/
so could be used by future sub-drivers
- Tweaked mailbox()
- Removed duplicate EC_MAILBOX_DATA_SIZE defs
- Removed some error messages / converted to dbg()

 drivers/platform/chrome/Kconfig            |   4 +-
 drivers/platform/chrome/Makefile           |   2 +
 drivers/platform/chrome/wilco_ec/Kconfig   |  24 +++
 drivers/platform/chrome/wilco_ec/Makefile  |   4 +
 drivers/platform/chrome/wilco_ec/core.c    |  99 +++++++++
 drivers/platform/chrome/wilco_ec/mailbox.c | 237 +++++++++++++++++++++
 include/linux/platform_data/wilco-ec.h     | 138 ++++++++++++
 7 files changed, 507 insertions(+), 1 deletion(-)
 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/mailbox.c
 create mode 100644 include/linux/platform_data/wilco-ec.h

diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 16b1615958aa..bf889adfd4ef 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -49,6 +49,8 @@ config CHROMEOS_TBMC
 	  To compile this driver as a module, choose M here: the
 	  module will be called chromeos_tbmc.
 
+source "drivers/platform/chrome/wilco_ec/Kconfig"
+
 config CROS_EC_CTL
         tristate
 
@@ -86,7 +88,7 @@ config CROS_EC_LPC
 
 config CROS_EC_LPC_MEC
 	bool "ChromeOS Embedded Controller LPC Microchip EC (MEC) variant"
-	depends on CROS_EC_LPC
+	depends on CROS_EC_LPC || WILCO_EC
 	default n
 	help
 	  If you say Y here, a variant LPC protocol for the Microchip EC
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index cd591bf872bb..7242ee2b13c5 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -13,3 +13,5 @@ cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC)	+= cros_ec_lpc_mec.o
 obj-$(CONFIG_CROS_EC_LPC)		+= cros_ec_lpcs.o
 obj-$(CONFIG_CROS_EC_PROTO)		+= cros_ec_proto.o
 obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT)	+= cros_kbd_led_backlight.o
+
+obj-$(CONFIG_WILCO_EC)			+= wilco_ec/
diff --git a/drivers/platform/chrome/wilco_ec/Kconfig b/drivers/platform/chrome/wilco_ec/Kconfig
new file mode 100644
index 000000000000..f8e6c9e8c5cd
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/Kconfig
@@ -0,0 +1,24 @@
+menuconfig WILCO_EC_PLATFORM
+	bool "Platform support for Wilco Embedded Controller hardware"
+	help
+	  Say Y here to get to see options for platform support for
+	  various Wilco EC features. This option alone does
+	  not add any kernel code.
+
+	  If you say N, all options in this submenu will be skipped and disabled.
+
+if WILCO_EC_PLATFORM
+
+config WILCO_EC
+	tristate "ChromeOS Wilco Embedded Controller"
+	depends on ACPI && X86
+	select CROS_EC_LPC_MEC
+	help
+	  If you say Y here, you get support for talking to the ChromeOS
+	  Wilco EC over an eSPI bus. This uses a simple byte-level protocol
+	  with a checksum.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called wilco_ec.
+
+endif # WILCO_EC_PLATFORM
diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
new file mode 100644
index 000000000000..03b32301dc61
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+
+wilco_ec-objs				:= core.o mailbox.o
+obj-$(CONFIG_WILCO_EC)			+= wilco_ec.o
diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
new file mode 100644
index 000000000000..2d1d8d3f9a94
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/core.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Mailbox interface for Wilco Embedded Controller
+ *
+ * Copyright 2018 Google LLC
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/platform_device.h>
+
+#include "../cros_ec_lpc_mec.h"
+
+static struct resource *wilco_get_resource(struct platform_device *pdev,
+					   int index)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_IO, index);
+	if (!res) {
+		dev_dbg(dev, "couldn't find IO resource %d\n", index);
+		return NULL;
+	}
+
+	if (!devm_request_region(dev, res->start, resource_size(res),
+				 dev_name(dev)))
+		return NULL;
+
+	return res;
+}
+
+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)
+		return -ENOMEM;
+	platform_set_drvdata(pdev, ec);
+	ec->dev = dev;
+	mutex_init(&ec->mailbox_lock);
+
+	/* Largest data buffer size requirement is extended data response */
+	ec->data_size = sizeof(struct wilco_ec_response) +
+		EC_MAILBOX_DATA_SIZE_EXTENDED;
+	ec->data_buffer = devm_kzalloc(dev, ec->data_size, GFP_KERNEL);
+	if (!ec->data_buffer)
+		return -ENOMEM;
+
+	/* Prepare access to IO regions provided by ACPI */
+	ec->io_data = wilco_get_resource(pdev, 0);	/* Host Data */
+	ec->io_command = wilco_get_resource(pdev, 1);	/* Host Command */
+	ec->io_packet = wilco_get_resource(pdev, 2);	/* MEC EMI */
+	if (!ec->io_data || !ec->io_command || !ec->io_packet)
+		return -ENODEV;
+
+	/* Initialize cros_ec register interface for communication */
+	cros_ec_lpc_mec_init(ec->io_packet->start,
+			     ec->io_packet->start + EC_MAILBOX_DATA_SIZE);
+
+	return 0;
+}
+
+static int wilco_ec_remove(struct platform_device *pdev)
+{
+	/* Teardown cros_ec interface */
+	cros_ec_lpc_mec_destroy();
+
+	return 0;
+}
+
+static const struct acpi_device_id wilco_ec_acpi_device_ids[] = {
+	{ "GOOG000C", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, wilco_ec_acpi_device_ids);
+
+static struct platform_driver wilco_ec_driver = {
+	.driver = {
+		.name = "wilco_ec",
+		.acpi_match_table = wilco_ec_acpi_device_ids,
+	},
+	.probe = wilco_ec_probe,
+	.remove = wilco_ec_remove,
+};
+
+module_platform_driver(wilco_ec_driver);
+
+MODULE_AUTHOR("Nick Crews <ncrews@google.com>");
+MODULE_AUTHOR("Duncan Laurie <dlaurie@chromium.org>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ChromeOS Wilco Embedded Controller driver");
+MODULE_ALIAS("platform:wilco-ec");
diff --git a/drivers/platform/chrome/wilco_ec/mailbox.c b/drivers/platform/chrome/wilco_ec/mailbox.c
new file mode 100644
index 000000000000..221de0d020cf
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/mailbox.c
@@ -0,0 +1,237 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Mailbox interface for Wilco Embedded Controller
+ *
+ * Copyright 2018 Google LLC
+ *
+ * The Wilco EC is similar to a typical ChromeOS embedded controller.
+ * It uses the same MEC based low-level communication and a similar
+ * protocol, but with some important differences.  The EC firmware does
+ * not support the same mailbox commands so it is not registered as a
+ * cros_ec device type.
+ *
+ * Most messages follow a standard format, but there are some exceptions
+ * and an interface is provided to do direct/raw transactions that do not
+ * make assumptions about byte placement.
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/platform_device.h>
+
+#include "../cros_ec_lpc_mec.h"
+
+/* Version of mailbox interface */
+#define EC_MAILBOX_VERSION		0
+
+/* Command to start mailbox transaction */
+#define EC_MAILBOX_START_COMMAND	0xda
+
+/* Version of EC protocol */
+#define EC_MAILBOX_PROTO_VERSION	3
+
+/* Number of header bytes to be counted as data bytes */
+#define EC_MAILBOX_DATA_EXTRA		2
+
+/* Maximum timeout */
+#define EC_MAILBOX_TIMEOUT		HZ
+
+/* EC response flags */
+#define EC_CMDR_DATA		BIT(0)	/* Data ready for host to read */
+#define EC_CMDR_PENDING		BIT(1)	/* Write pending to EC */
+#define EC_CMDR_BUSY		BIT(2)	/* EC is busy processing a command */
+#define EC_CMDR_CMD		BIT(3)	/* Last host write was a command */
+
+/**
+ * wilco_ec_response_timed_out() - Wait for EC response.
+ * @ec: EC device.
+ *
+ * Return: true if EC timed out, false if EC did not time out.
+ */
+static bool wilco_ec_response_timed_out(struct wilco_ec_device *ec)
+{
+	unsigned long timeout = jiffies + EC_MAILBOX_TIMEOUT;
+
+	usleep_range(200, 300);
+	do {
+		if (!(inb(ec->io_command->start) &
+		      (EC_CMDR_PENDING | EC_CMDR_BUSY)))
+			return false;
+		usleep_range(100, 200);
+	} while (time_before(jiffies, timeout));
+
+	return true;
+}
+
+/**
+ * wilco_ec_checksum() - Compute 8bit checksum over data range.
+ * @data: Data to checksum.
+ * @size: Number of bytes to checksum.
+ *
+ * Return: 8bit checksum of provided data.
+ */
+static u8 wilco_ec_checksum(const void *data, size_t size)
+{
+	u8 *data_bytes = (u8 *)data;
+	u8 checksum = 0;
+	size_t i;
+
+	for (i = 0; i < size; i++)
+		checksum += data_bytes[i];
+
+	return checksum;
+}
+
+/**
+ * wilco_ec_prepare() - Prepare the request structure for the EC.
+ * @msg: EC message with request information.
+ * @rq: EC request structure to fill.
+ */
+static void wilco_ec_prepare(struct wilco_ec_message *msg,
+			     struct wilco_ec_request *rq)
+{
+	memset(rq, 0, sizeof(*rq));
+
+	/* Handle messages without trimming bytes from the request */
+	if (msg->request_size && msg->flags & WILCO_EC_FLAG_RAW_REQUEST) {
+		rq->reserved_raw = *(u8 *)msg->request_data;
+		msg->request_size--;
+		memmove(msg->request_data, msg->request_data + 1,
+			msg->request_size);
+	}
+
+	/* Fill in request packet */
+	rq->struct_version = EC_MAILBOX_PROTO_VERSION;
+	rq->mailbox_id = msg->type;
+	rq->mailbox_version = EC_MAILBOX_VERSION;
+	rq->data_size = msg->request_size + EC_MAILBOX_DATA_EXTRA;
+	rq->command = msg->command;
+
+	/* Checksum header and data */
+	rq->checksum = wilco_ec_checksum(rq, sizeof(*rq));
+	rq->checksum += wilco_ec_checksum(msg->request_data, msg->request_size);
+	rq->checksum = -rq->checksum;
+}
+
+/**
+ * wilco_ec_transfer() - Perform actual data transfer.
+ * @ec: EC device.
+ * @msg: EC message data for request and response.
+ * @rq: Filled in request structure
+ *
+ * Context: ec->mailbox_lock should be held while using this function.
+ * Return: number of bytes received or negative error code on failure.
+ */
+int wilco_ec_transfer(struct wilco_ec_device *ec, struct wilco_ec_message *msg,
+		      struct wilco_ec_request *rq)
+{
+	struct wilco_ec_response *rs;
+	u8 checksum;
+	u8 flag;
+	size_t size;
+
+	/* Write request header, then data */
+	cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, 0, sizeof(*rq), (u8 *)rq);
+	cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, sizeof(*rq), msg->request_size,
+				 msg->request_data);
+
+	/* Start the command */
+	outb(EC_MAILBOX_START_COMMAND, ec->io_command->start);
+
+	/* Wait for it to complete */
+	if (wilco_ec_response_timed_out(ec)) {
+		dev_dbg(ec->dev, "response timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	/* For some commands (eg shutdown) the EC will not respond, that's OK */
+	if (msg->flags & WILCO_EC_FLAG_NO_RESPONSE) {
+		dev_dbg(ec->dev, "EC does not respond to this command\n");
+		return 0;
+	}
+
+	/* Check result */
+	flag = inb(ec->io_data->start);
+	if (flag) {
+		dev_dbg(ec->dev, "bad response: 0x%02x\n", flag);
+		return -EIO;
+	}
+
+	if (msg->flags & WILCO_EC_FLAG_EXTENDED_DATA)
+		size = EC_MAILBOX_DATA_SIZE_EXTENDED;
+	else
+		size = EC_MAILBOX_DATA_SIZE;
+
+	/* Read back response */
+	rs = ec->data_buffer;
+	checksum = cros_ec_lpc_io_bytes_mec(MEC_IO_READ, 0,
+					    sizeof(*rs) + size, (u8 *)rs);
+	if (checksum) {
+		dev_dbg(ec->dev, "bad packet checksum 0x%02x\n", rs->checksum);
+		return -EBADMSG;
+	}
+
+	/* Check that the EC reported success */
+	msg->result = rs->result;
+	if (msg->result) {
+		dev_dbg(ec->dev, "bad response: 0x%02x\n", msg->result);
+		return -EBADMSG;
+	}
+
+	/* Check the returned data size, skipping the header */
+	if (rs->data_size != size) {
+		dev_dbg(ec->dev, "unexpected packet size (%u != %zu)",
+			rs->data_size, size);
+		return -EMSGSIZE;
+	}
+
+	/* Skip 1 response data byte unless specified */
+	size = (msg->flags & WILCO_EC_FLAG_RAW_RESPONSE) ? 0 : 1;
+	if ((ssize_t) rs->data_size - size < msg->response_size) {
+		dev_dbg(ec->dev, "response data too short (%zd < %zu)",
+			(ssize_t) rs->data_size - size, msg->response_size);
+		return -EMSGSIZE;
+	}
+
+	/* Ignore response data bytes as requested */
+	memcpy(msg->response_data, rs->data + size, msg->response_size);
+
+	/* Return actual amount of data received */
+	return msg->response_size;
+}
+
+/**
+ * wilco_ec_mailbox() - Send EC request and receive EC response.
+ * @ec: EC device.
+ * @msg: EC message data for request and response.
+ *
+ * On entry msg->type, msg->flags, msg->command, msg->request_size,
+ * msg->response_size, and msg->request_data should all be filled in.
+ *
+ * On exit msg->result and msg->response_data will be filled.
+ *
+ * Return: number of bytes received or negative error code on failure.
+ */
+int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg)
+{
+	struct wilco_ec_request *rq;
+	int ret;
+
+	dev_dbg(ec->dev, "cmd=%02x type=%04x flags=%02x rslen=%zu rqlen=%zu\n",
+		msg->command, msg->type, msg->flags, msg->response_size,
+		msg->request_size);
+
+	/* Prepare request packet */
+	rq = ec->data_buffer;
+	wilco_ec_prepare(msg, rq);
+
+	mutex_lock(&ec->mailbox_lock);
+	ret = wilco_ec_transfer(ec, msg, rq);
+	mutex_unlock(&ec->mailbox_lock);
+
+	return ret;
+
+}
+EXPORT_SYMBOL_GPL(wilco_ec_mailbox);
diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
new file mode 100644
index 000000000000..5477b8802f81
--- /dev/null
+++ b/include/linux/platform_data/wilco-ec.h
@@ -0,0 +1,138 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * ChromeOS Wilco Embedded Controller
+ *
+ * Copyright 2018 Google LLC
+ */
+
+#ifndef WILCO_EC_H
+#define WILCO_EC_H
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+
+#define WILCO_EC_FLAG_NO_RESPONSE	BIT(0) /* EC does not respond */
+#define WILCO_EC_FLAG_EXTENDED_DATA	BIT(1) /* EC returns 256 data bytes */
+#define WILCO_EC_FLAG_RAW_REQUEST	BIT(2) /* Do not trim request data */
+#define WILCO_EC_FLAG_RAW_RESPONSE	BIT(3) /* Do not trim response data */
+#define WILCO_EC_FLAG_RAW		(WILCO_EC_FLAG_RAW_REQUEST | \
+					 WILCO_EC_FLAG_RAW_RESPONSE)
+
+/* Normal commands have a maximum 32 bytes of data */
+#define EC_MAILBOX_DATA_SIZE		32
+
+/* Extended commands have 256 bytes of response data */
+#define EC_MAILBOX_DATA_SIZE_EXTENDED	256
+
+/**
+ * struct wilco_ec_device - Wilco Embedded Controller handle.
+ * @dev: Device handle.
+ * @mailbox_lock: Mutex to ensure one mailbox command at a time.
+ * @io_command: I/O port for mailbox command.  Provided by ACPI.
+ * @io_data: I/O port for mailbox data.  Provided by ACPI.
+ * @io_packet: I/O port for mailbox packet data.  Provided by ACPI.
+ * @data_buffer: Buffer used for EC communication.  The same buffer
+ *               is used to hold the request and the response.
+ * @data_size: Size of the data buffer used for EC communication.
+ */
+struct wilco_ec_device {
+	struct device *dev;
+	struct mutex mailbox_lock;
+	struct resource *io_command;
+	struct resource *io_data;
+	struct resource *io_packet;
+	void *data_buffer;
+	size_t data_size;
+};
+
+/**
+ * enum wilco_ec_msg_type - Message type to select a set of command codes.
+ * @WILCO_EC_MSG_LEGACY: Legacy EC messages for standard EC behavior.
+ * @WILCO_EC_MSG_PROPERTY: Get/Set/Sync EC controlled NVRAM property.
+ * @WILCO_EC_MSG_TELEMETRY: Telemetry data provided by the EC.
+ */
+enum wilco_ec_msg_type {
+	WILCO_EC_MSG_LEGACY = 0x00f0,
+	WILCO_EC_MSG_PROPERTY = 0x00f2,
+	WILCO_EC_MSG_TELEMETRY = 0x00f5,
+};
+
+/**
+ * struct wilco_ec_message - Request and response message.
+ * @type: Mailbox message type.
+ * @flags: Message flags.
+ * @command: Mailbox command code.
+ * @result: Result code from the EC.  Non-zero indicates an error.
+ * @request_size: Number of bytes to send to the EC.
+ * @request_data: Buffer containing the request data.
+ * @response_size: Number of bytes expected from the EC.
+ *                 This is 32 by default and 256 if the flag
+ *                 is set for %WILCO_EC_FLAG_EXTENDED_DATA
+ * @response_data: Buffer containing the response data, should be
+ *                 response_size bytes and allocated by caller.
+ */
+struct wilco_ec_message {
+	enum wilco_ec_msg_type type;
+	u8 flags;
+	u8 command;
+	u8 result;
+	size_t request_size;
+	void *request_data;
+	size_t response_size;
+	void *response_data;
+};
+
+/**
+ * struct wilco_ec_request - Mailbox request message format.
+ * @struct_version: Should be %EC_MAILBOX_PROTO_VERSION
+ * @checksum: Sum of all bytes must be 0.
+ * @mailbox_id: Mailbox identifier, specifies the command set.
+ * @mailbox_version: Mailbox interface version %EC_MAILBOX_VERSION
+ * @reserved: Set to zero.
+ * @data_size: Length of request, data + last 2 bytes of the header.
+ * @command: Mailbox command code, unique for each mailbox_id set.
+ * @reserved_raw: Set to zero for most commands, but is used by
+ *                some command types and for raw commands.
+ */
+struct wilco_ec_request {
+	u8 struct_version;
+	u8 checksum;
+	u16 mailbox_id;
+	u8 mailbox_version;
+	u8 reserved;
+	u16 data_size;
+	u8 command;
+	u8 reserved_raw;
+} __packed;
+
+/**
+ * struct wilco_ec_response - Mailbox response message format.
+ * @struct_version: Should be %EC_MAILBOX_PROTO_VERSION
+ * @checksum: Sum of all bytes must be 0.
+ * @result: Result code from the EC.  Non-zero indicates an error.
+ * @data_size: Length of the response data buffer.
+ * @reserved: Set to zero.
+ * @mbox0: EC returned data at offset 0 is unused (always 0) so this byte
+ *         is treated as part of the header instead of the data.
+ * @data: Response data buffer.  Max size is %EC_MAILBOX_DATA_SIZE_EXTENDED.
+ */
+struct wilco_ec_response {
+	u8 struct_version;
+	u8 checksum;
+	u16 result;
+	u16 data_size;
+	u8 reserved[2];
+	u8 mbox0;
+	u8 data[0];
+} __packed;
+
+/**
+ * wilco_ec_mailbox() - Send request to the EC and receive the response.
+ * @ec: Wilco EC device.
+ * @msg: Wilco EC message.
+ *
+ * Return: Number of bytes received or negative error code on failure.
+ */
+int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg);
+
+#endif /* WILCO_EC_H */
-- 
2.20.1.97.g81188d93c3-goog


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

* [PATCH v2 3/9] platform/chrome: Add sysfs attributes
  2019-01-14 22:03 [PATCH v2 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
  2019-01-14 22:03 ` [PATCH v2 1/9] platform/chrome: Remove cros_ec dependency in lpc_mec Nick Crews
  2019-01-14 22:03 ` [PATCH v2 2/9] platform/chrome: Add new driver for Wilco EC Nick Crews
@ 2019-01-14 22:03 ` Nick Crews
  2019-01-15 19:32   ` Enric Balletbo Serra
  2019-01-14 22:03 ` [PATCH v2 4/9] platform/chrome: Add support for raw commands in sysfs Nick Crews
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Nick Crews @ 2019-01-14 22:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: groeck, sjg, dlaurie, Duncan Laurie, Nick Crews,
	Enric Balletbo i Serra, Benson Leung

From: Duncan Laurie <dlaurie@google.com>

Add some sample sysfs attributes for the Wilco EC that show how
the mailbox interface works. "Legacy" attributes are those that
existed in the EC before it was adapted to ChromeOS.

> cat /sys/bus/platform/devices/GOOG000C\:00/version
Label        : 99.99.99
SVN Revision : 738ed.99
Model Number : 08;8
Build Date   : 08/30/18

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

Changes in v2:
- 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 in sysfs_init() and its caller

 .../ABI/testing/sysfs-platform-wilcoec        | 26 ++++++
 drivers/platform/chrome/wilco_ec/Makefile     |  2 +-
 drivers/platform/chrome/wilco_ec/core.c       | 15 +++
 drivers/platform/chrome/wilco_ec/legacy.c     | 91 +++++++++++++++++++
 drivers/platform/chrome/wilco_ec/legacy.h     | 47 ++++++++++
 drivers/platform/chrome/wilco_ec/sysfs.c      | 66 ++++++++++++++
 include/linux/platform_data/wilco-ec.h        | 14 +++
 7 files changed, 260 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-platform-wilcoec
 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/sysfs.c

diff --git a/Documentation/ABI/testing/sysfs-platform-wilcoec b/Documentation/ABI/testing/sysfs-platform-wilcoec
new file mode 100644
index 000000000000..09552338c160
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-platform-wilcoec
@@ -0,0 +1,26 @@
+What:		/sys/bus/platform/devices/GOOG000C\:00/version
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Display Wilco Embedded Controller version info
+
+		Output will be similar to the example below:
+		Label        : 95.00.06
+		SVN Revision : 5960a.06
+		Model Number : 08;8
+		Build Date   : 11/29/18
+
+		Although this contains multiple pieces of data in a single sysfs
+		attribute (inconsistent with normal sysfs conventions),
+		this format was chosen to be consistent with the version
+		attribute in /sys/class/chomeos/<device>/version
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/stealth_mode
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Turn stealth_mode on or off on EC. Stealth mode means that the
+		various LEDs, the LCD backlight, and onboard speakers are turned
+		off and remain off even if activated from the off state.
+		External monitors connected to the system are not affected.
+		In addition Wireless devices are turned off.
diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
index 03b32301dc61..7965a39e2e7a 100644
--- a/drivers/platform/chrome/wilco_ec/Makefile
+++ b/drivers/platform/chrome/wilco_ec/Makefile
@@ -1,4 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0
 
-wilco_ec-objs				:= core.o mailbox.o
+wilco_ec-objs				:= core.o mailbox.o sysfs.o legacy.o
 obj-$(CONFIG_WILCO_EC)			+= wilco_ec.o
diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
index 2d1d8d3f9a94..3193e8b2ec91 100644
--- a/drivers/platform/chrome/wilco_ec/core.c
+++ b/drivers/platform/chrome/wilco_ec/core.c
@@ -64,11 +64,26 @@ static int wilco_ec_probe(struct platform_device *pdev)
 	cros_ec_lpc_mec_init(ec->io_packet->start,
 			     ec->io_packet->start + EC_MAILBOX_DATA_SIZE);
 
+	/* Create sysfs attributes for userspace interaction */
+	ret = wilco_ec_sysfs_init(ec);
+	if (ret < 0) {
+		dev_err(dev, "Failed to create sysfs attributes\n");
+		goto destroy_lpc_mec;
+	}
 	return 0;
+
+destroy_lpc_mec:
+	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);
+
+	/* Remove sysfs attributes */
+	wilco_ec_sysfs_remove(ec);
+
 	/* Teardown cros_ec interface */
 	cros_ec_lpc_mec_destroy();
 
diff --git a/drivers/platform/chrome/wilco_ec/legacy.c b/drivers/platform/chrome/wilco_ec/legacy.c
new file mode 100644
index 000000000000..1fdf94d22f57
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/legacy.c
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Legacy (non-Chrome-specific) sysfs attributes for Wilco EC
+ *
+ * Copyright 2018 Google LLC
+ */
+
+#include <linux/ctype.h>
+#include <linux/device.h>
+#include <linux/platform_data/wilco-ec.h>
+
+#include "legacy.h"
+
+struct ec_info {
+	u8 index;
+	const char *label;
+};
+
+static ssize_t wilco_ec_show_info(struct wilco_ec_device *ec, char *buf,
+				  ssize_t count, struct ec_info *info)
+{
+	char result[EC_INFO_SIZE];
+	struct wilco_ec_message msg = {
+		.type = WILCO_EC_MSG_LEGACY,
+		.command = EC_COMMAND_EC_INFO,
+		.request_data = &info->index,
+		.request_size = sizeof(info->index),
+		.response_data = result,
+		.response_size = EC_INFO_SIZE,
+	};
+	int ret;
+
+	ret = wilco_ec_mailbox(ec, &msg);
+	if (ret != EC_INFO_SIZE)
+		return scnprintf(buf + count, PAGE_SIZE - count,
+				 "%-12s : ERROR %d\n", info->label, ret);
+
+	return scnprintf(buf + count, PAGE_SIZE - count,
+			 "%-12s : %s\n", info->label, result);
+}
+
+ssize_t wilco_ec_version_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct wilco_ec_device *ec = dev_get_drvdata(dev);
+	struct ec_info wilco_ec_info[] = {
+		{ 0, "Label" },
+		{ 1, "SVN Revision" },
+		{ 2, "Model Number" },
+		{ 3, "Build Date" },
+		{ 0xff, NULL },
+	};
+	struct ec_info *info = wilco_ec_info;
+	ssize_t c = 0;
+
+	for (info = wilco_ec_info; info->label; info++)
+		c += wilco_ec_show_info(ec, buf, c, info);
+
+	return c;
+}
+
+ssize_t wilco_ec_stealth_mode_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count)
+{
+	struct wilco_ec_device *ec = dev_get_drvdata(dev);
+	u8 param;
+	struct wilco_ec_message msg = {
+		.type = WILCO_EC_MSG_LEGACY,
+		.command = EC_COMMAND_STEALTH_MODE,
+		.request_data = &param,
+		.request_size = sizeof(param),
+	};
+	int ret;
+	bool enable;
+
+	ret = kstrtobool(buf, &enable);
+	if (ret) {
+		dev_err(dev, "Unable to parse '%s' to bool", buf);
+		return ret;
+	}
+
+	/* Invert input parameter, EC expects 0=on and 1=off */
+	param = !enable;
+
+	ret = wilco_ec_mailbox(ec, &msg);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
diff --git a/drivers/platform/chrome/wilco_ec/legacy.h b/drivers/platform/chrome/wilco_ec/legacy.h
new file mode 100644
index 000000000000..5c857cb57fa9
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/legacy.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Legacy (non-Chrome-specific) sysfs attributes for Wilco EC
+ *
+ * Copyright 2018 Google LLC
+ */
+
+#ifndef WILCO_EC_LEGACY_H
+#define WILCO_EC_LEGACY_H
+
+#include <linux/device.h>
+
+#define EC_COMMAND_EC_INFO		0x38
+#define EC_INFO_SIZE			9
+#define EC_COMMAND_STEALTH_MODE		0xfc
+
+/**
+ * wilco_ec_version_show() - Display Wilco Embedded Controller version info
+ *
+ * Output will be similar to the example below:
+ * Label        : 95.00.06
+ * SVN Revision : 5960a.06
+ * Model Number : 08;8
+ * Build Date   : 11/29/18
+ *
+ * Although this contains multiple pieces of data in a single sysfs attribute
+ * (inconsistent with normal sysfs conventions), this format was chosen to be
+ * consistent with the version attribute in ../cros_ec_sysfs.c
+ */
+ssize_t wilco_ec_version_show(struct device *dev, struct device_attribute *attr,
+			      char *buf);
+
+/**
+ * wilco_ec_stealth_mode_store() - Turn stealth_mode on or off on EC
+ * @dev: Device representing the EC
+ * @attr: The attribute in question
+ * @buf: Input buffer, should be parseable by kstrtobool(). Anything parsed to
+ *	 True means enable stealth mode (turn off screen, etc)
+ * @count: Number of bytes in input buffer
+ *
+ * Return: Number of bytes consumed from input, negative error code on failure
+ */
+ssize_t wilco_ec_stealth_mode_store(struct device *dev,
+				    struct device_attribute *attr,
+				    const char *buf, size_t count);
+
+#endif /* WILCO_EC_LEGACY_H */
diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c b/drivers/platform/chrome/wilco_ec/sysfs.c
new file mode 100644
index 000000000000..e78a3ec3506b
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/sysfs.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sysfs attributes for Wilco Embedded Controller
+ *
+ * Copyright 2018 Google LLC
+ *
+ * The sysfs attributes appear under /sys/bus/platform/devices/GOOG000C\:00/
+ * To actually learn what each attribute does, read the corresponding _show() or
+ * _store() function source.
+ */
+
+#include <linux/ctype.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/platform_device.h>
+#include <linux/sysfs.h>
+
+#include "legacy.h"
+
+#define WILCO_EC_ATTR_RO(_name)						\
+__ATTR(_name, 0444, wilco_ec_##_name##_show, NULL)
+
+#define WILCO_EC_ATTR_WO(_name)						\
+__ATTR(_name, 0200, NULL, wilco_ec_##_name##_store)
+
+#define WILCO_EC_ATTR_RW(_name)						\
+__ATTR(_name, 0644, wilco_ec_##_name##_show, wilco_ec_##_name##_store)
+
+/* Make top-level attributes, which will live inside GOOG000C:00/ */
+static struct device_attribute version_attr = WILCO_EC_ATTR_RO(version);
+static struct device_attribute stealth_attr = WILCO_EC_ATTR_WO(stealth_mode);
+static struct attribute *wilco_ec_toplevel_attrs[] = {
+	&version_attr.attr,
+	&stealth_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(wilco_ec_toplevel);
+
+/**
+ * wilco_ec_sysfs_init() - Initialize the sysfs directories and attributes
+ * @dev: The device representing the EC
+ *
+ * Creates the sysfs directory structure and populates it with all attributes.
+ * If there is a problem it will clean up the entire filesystem.
+ *
+ * Return 0 on success, -ENOMEM on failure creating directories or attibutes.
+ */
+int wilco_ec_sysfs_init(struct wilco_ec_device *ec)
+{
+	struct device *dev = ec->dev;
+	int ret;
+
+	/* add the top-level attributes */
+	ret = sysfs_create_groups(&dev->kobj, wilco_ec_toplevel_groups);
+	if (ret)
+		return -ENOMEM;
+
+	return 0;
+}
+
+void wilco_ec_sysfs_remove(struct wilco_ec_device *ec)
+{
+	struct device *dev = ec->dev;
+
+	/* go upwards through the directory structure */
+	sysfs_remove_groups(&dev->kobj, wilco_ec_toplevel_groups);
+}
diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
index 5477b8802f81..7c6ab6de7239 100644
--- a/include/linux/platform_data/wilco-ec.h
+++ b/include/linux/platform_data/wilco-ec.h
@@ -135,4 +135,18 @@ struct wilco_ec_response {
  */
 int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg);
 
+/**
+ * wilco_ec_sysfs_init() - Create sysfs attributes.
+ * @ec: EC device.
+ *
+ * Return: 0 for success or negative error code on failure.
+ */
+int wilco_ec_sysfs_init(struct wilco_ec_device *ec);
+
+/**
+ * wilco_ec_sysfs_remove() - Remove sysfs attributes.
+ * @ec: EC device.
+ */
+void wilco_ec_sysfs_remove(struct wilco_ec_device *ec);
+
 #endif /* WILCO_EC_H */
-- 
2.20.1.97.g81188d93c3-goog


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

* [PATCH v2 4/9] platform/chrome: Add support for raw commands in sysfs
  2019-01-14 22:03 [PATCH v2 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
                   ` (2 preceding siblings ...)
  2019-01-14 22:03 ` [PATCH v2 3/9] platform/chrome: Add sysfs attributes Nick Crews
@ 2019-01-14 22:03 ` Nick Crews
  2019-01-15 19:37   ` Enric Balletbo Serra
  2019-01-14 22:03 ` [PATCH v2 5/9] platform/chrome: rtc: Add RTC driver for Wilco EC Nick Crews
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Nick Crews @ 2019-01-14 22:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: groeck, sjg, dlaurie, Duncan Laurie, Nick Crews,
	Enric Balletbo i Serra, Benson Leung

From: Duncan Laurie <dlaurie@google.com>

Add a sysfs attribute that allows sending raw commands to the EC.
This is useful for development and debug but should not be enabled
in a production environment.

> echo 00 f0 38 00 03 00 > /sys/bus/platform/devices/GOOG000C\:00/raw
> cat /sys/bus/platform/devices/GOOG000C\:00/raw
00 37 33 38 65 64 00...

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

Changes in v2:
- Add sysfs documentation
- rm duplicate EC_MAILBOX_DATA_SIZE defs
- Make docstrings follow kernel style
- Fix tags in commit msg
- Reading raw now includes ASCII translation

 .../ABI/testing/sysfs-platform-wilcoec        | 21 ++++
 drivers/platform/chrome/wilco_ec/Kconfig      |  9 ++
 drivers/platform/chrome/wilco_ec/legacy.c     | 99 +++++++++++++++++++
 drivers/platform/chrome/wilco_ec/legacy.h     | 46 +++++++++
 drivers/platform/chrome/wilco_ec/sysfs.c      |  6 ++
 5 files changed, 181 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-platform-wilcoec b/Documentation/ABI/testing/sysfs-platform-wilcoec
index 09552338c160..cac2cf11835f 100644
--- a/Documentation/ABI/testing/sysfs-platform-wilcoec
+++ b/Documentation/ABI/testing/sysfs-platform-wilcoec
@@ -24,3 +24,24 @@ Description:
 		off and remain off even if activated from the off state.
 		External monitors connected to the system are not affected.
 		In addition Wireless devices are turned off.
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/raw
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Write and read raw mailbox commands to the EC.
+
+		For writing:
+		Bytes 0-1 indicate the message type:
+			00 F0 = Execute Legacy Command
+			00 F2 = Read/Write NVRAM Property
+		Byte 2 provides the command code
+		Bytes 3+ consist of the data passed in the request
+
+		Example:
+		// Request EC info type 3 (EC firmware build date)
+		$ echo 00 f0 38 00 03 00 > raw
+		// View the result. The decoded ASCII result "12/21/18" is
+		// included after the raw hex.
+		$ cat raw
+		00 31 32 2f 32 31 2f 31 38 00 38 00 01 00 2f 00  .12/21/18.8...
diff --git a/drivers/platform/chrome/wilco_ec/Kconfig b/drivers/platform/chrome/wilco_ec/Kconfig
index f8e6c9e8c5cd..0bd84c98b79b 100644
--- a/drivers/platform/chrome/wilco_ec/Kconfig
+++ b/drivers/platform/chrome/wilco_ec/Kconfig
@@ -21,4 +21,13 @@ config WILCO_EC
 	  To compile this driver as a module, choose M here: the
 	  module will be called wilco_ec.
 
+config WILCO_EC_SYSFS_RAW
+	bool "Enable raw access to EC via sysfs"
+	depends on WILCO_EC
+	help
+	  If you say Y here, you get support for sending raw commands to
+	  the Wilco EC via sysfs.  These commands do not do any byte
+	  manipulation and allow for testing arbitrary commands.  This
+	  interface is intended for debug only and is disabled by default.
+
 endif # WILCO_EC_PLATFORM
diff --git a/drivers/platform/chrome/wilco_ec/legacy.c b/drivers/platform/chrome/wilco_ec/legacy.c
index 1fdf94d22f57..79bac5dccb39 100644
--- a/drivers/platform/chrome/wilco_ec/legacy.c
+++ b/drivers/platform/chrome/wilco_ec/legacy.c
@@ -11,6 +11,105 @@
 
 #include "legacy.h"
 
+#ifdef CONFIG_WILCO_EC_SYSFS_RAW
+
+/* Raw data buffer, large enough to hold extended responses */
+static size_t raw_response_size;
+static u8 raw_response_data[EC_MAILBOX_DATA_SIZE_EXTENDED];
+
+ssize_t wilco_ec_raw_store(struct device *dev, struct device_attribute *attr,
+			   const char *buf, size_t count)
+{
+	struct wilco_ec_device *ec = dev_get_drvdata(dev);
+	struct wilco_ec_message msg;
+	u8 raw_request_data[EC_MAILBOX_DATA_SIZE];
+	int in_offset = 0;
+	int out_offset = 0;
+	int ret;
+
+	while (in_offset < count) {
+		char word_buf[EC_MAILBOX_DATA_SIZE];
+		u8 byte;
+		int start_offset = in_offset;
+		int end_offset;
+
+		/* Find the start of the byte */
+		while (buf[start_offset] && isspace(buf[start_offset]))
+			start_offset++;
+		if (!buf[start_offset])
+			break;
+
+		/* Find the start of the next byte, if any */
+		end_offset = start_offset;
+		while (buf[end_offset] && !isspace(buf[end_offset]))
+			end_offset++;
+		if (start_offset > count || end_offset > count)
+			break;
+		if (start_offset > EC_MAILBOX_DATA_SIZE ||
+		    end_offset > EC_MAILBOX_DATA_SIZE)
+			break;
+
+		/* Copy to a new NULL terminated string */
+		memcpy(word_buf, buf + start_offset, end_offset - start_offset);
+		word_buf[end_offset - start_offset] = '\0';
+
+		/* Convert from hex string */
+		ret = kstrtou8(word_buf, 16, &byte);
+		if (ret)
+			break;
+
+		/* Fill this byte into the request buffer */
+		raw_request_data[out_offset++] = byte;
+		if (out_offset >= EC_MAILBOX_DATA_SIZE)
+			break;
+
+		in_offset = end_offset;
+	}
+	if (out_offset == 0)
+		return -EINVAL;
+
+	/* Clear response data buffer */
+	memset(raw_response_data, '\0', EC_MAILBOX_DATA_SIZE_EXTENDED);
+
+	msg.type = raw_request_data[0] << 8 | raw_request_data[1];
+	msg.flags = WILCO_EC_FLAG_RAW;
+	msg.command = raw_request_data[2];
+	msg.request_data = raw_request_data + 3;
+	msg.request_size = out_offset - 3;
+	msg.response_data = raw_response_data;
+	msg.response_size = EC_MAILBOX_DATA_SIZE;
+
+	/* Telemetry commands use extended response data */
+	if (msg.type == WILCO_EC_MSG_TELEMETRY) {
+		msg.flags |= WILCO_EC_FLAG_EXTENDED_DATA;
+		msg.response_size = EC_MAILBOX_DATA_SIZE_EXTENDED;
+	}
+
+	ret = wilco_ec_mailbox(ec, &msg);
+	if (ret < 0)
+		return ret;
+	raw_response_size = ret;
+	return count;
+}
+
+ssize_t wilco_ec_raw_show(struct device *dev, struct device_attribute *attr,
+			  char *buf)
+{
+	ssize_t count = 0;
+
+	if (raw_response_size) {
+		count = hex_dump_to_buffer(raw_response_data, raw_response_size,
+					   16, 1, buf, PAGE_SIZE, true);
+
+		/* Only return response the first time it is read */
+		raw_response_size = 0;
+	}
+
+	return count;
+}
+
+#endif /* CONFIG_WILCO_EC_SYSFS_RAW */
+
 struct ec_info {
 	u8 index;
 	const char *label;
diff --git a/drivers/platform/chrome/wilco_ec/legacy.h b/drivers/platform/chrome/wilco_ec/legacy.h
index 5c857cb57fa9..340787d25d44 100644
--- a/drivers/platform/chrome/wilco_ec/legacy.h
+++ b/drivers/platform/chrome/wilco_ec/legacy.h
@@ -14,6 +14,52 @@
 #define EC_INFO_SIZE			9
 #define EC_COMMAND_STEALTH_MODE		0xfc
 
+#ifdef CONFIG_WILCO_EC_SYSFS_RAW
+
+/**
+ * wilco_ec_raw_store() - Write a raw command to EC, store result to view later
+ * @dev: Device representing the EC
+ * @attr: The attribute in question
+ * @buf: Input buffer, format explained below
+ * @count: Number of bytes in input buffer
+ *
+ * Bytes 0-1 indicate the message type:
+ *  00 F0 = Execute Legacy Command
+ *  00 F2 = Read/Write NVRAM Property
+ * Byte 2 provides the command code
+ * Bytes 3+ consist of the data passed in the request
+ *
+ * example: read the EC info type 3 (EC firmware build date):
+ *  # echo 00 f0 38 00 03 00 > raw
+ *
+ * After calling this function, read the result by using raw_show()
+ *
+ * Return: Number of bytes consumed from input, negative error code on failure
+ */
+ssize_t wilco_ec_raw_store(struct device *dev, struct device_attribute *attr,
+			   const char *buf, size_t count);
+
+/**
+ * wilco_ec_raw_show() - Show result from previous call to raw_store()
+ * @dev: Device representing the EC
+ * @attr: The attribute in question
+ * @buf: Output buffer to be filled
+ *
+ * Example usage:
+ *	// Call raw_store(), read EC info type 3 (EC firmware build date)
+ *	# echo 00 f0 38 00 03 00 > raw
+ *	// Call this function, view the result. The decoded ASCII result
+ *	// "12/21/18" is included after the raw hex.
+ *	# cat raw
+ *	00 31 32 2f 32 31 2f 31 38 00 38 00 01 00 2f 00  .12/21/18.8.../.
+ *
+ * Return: Number of bytes written to output, negative error code on failure
+ */
+ssize_t wilco_ec_raw_show(struct device *dev, struct device_attribute *attr,
+			  char *buf);
+
+#endif /* CONFIG_WILCO_EC_SYSFS_RAW */
+
 /**
  * wilco_ec_version_show() - Display Wilco Embedded Controller version info
  *
diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c b/drivers/platform/chrome/wilco_ec/sysfs.c
index e78a3ec3506b..0611a73fcdce 100644
--- a/drivers/platform/chrome/wilco_ec/sysfs.c
+++ b/drivers/platform/chrome/wilco_ec/sysfs.c
@@ -28,9 +28,15 @@ __ATTR(_name, 0644, wilco_ec_##_name##_show, wilco_ec_##_name##_store)
 /* Make top-level attributes, which will live inside GOOG000C:00/ */
 static struct device_attribute version_attr = WILCO_EC_ATTR_RO(version);
 static struct device_attribute stealth_attr = WILCO_EC_ATTR_WO(stealth_mode);
+#ifdef CONFIG_WILCO_EC_SYSFS_RAW
+static struct device_attribute raw_attr = WILCO_EC_ATTR_RW(raw);
+#endif
 static struct attribute *wilco_ec_toplevel_attrs[] = {
 	&version_attr.attr,
 	&stealth_attr.attr,
+#ifdef CONFIG_WILCO_EC_SYSFS_RAW
+	&raw_attr.attr,
+#endif
 	NULL
 };
 ATTRIBUTE_GROUPS(wilco_ec_toplevel);
-- 
2.20.1.97.g81188d93c3-goog


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

* [PATCH v2 5/9] platform/chrome: rtc: Add RTC driver for Wilco EC
  2019-01-14 22:03 [PATCH v2 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
                   ` (3 preceding siblings ...)
  2019-01-14 22:03 ` [PATCH v2 4/9] platform/chrome: Add support for raw commands in sysfs Nick Crews
@ 2019-01-14 22:03 ` Nick Crews
  2019-01-14 22:25   ` Alexandre Belloni
  2019-01-14 22:03 ` [PATCH v2 6/9] platform/chrome: Add event handling Nick Crews
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Nick Crews @ 2019-01-14 22:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: groeck, sjg, dlaurie, Duncan Laurie, Nick Crews, linux-rtc,
	Enric Balletbo i Serra, Alessandro Zummo, Benson Leung,
	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@google.com>
---

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
- rm unused rtc_sync() function

 drivers/platform/chrome/wilco_ec/core.c |  18 +++
 drivers/rtc/Kconfig                     |  11 ++
 drivers/rtc/Makefile                    |   1 +
 drivers/rtc/rtc-wilco-ec.c              | 178 ++++++++++++++++++++++++
 4 files changed, 208 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 3193e8b2ec91..61dacd582582 100644
--- a/drivers/platform/chrome/wilco_ec/core.c
+++ b/drivers/platform/chrome/wilco_ec/core.c
@@ -37,6 +37,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);
@@ -70,8 +71,25 @@ static int wilco_ec_probe(struct platform_device *pdev)
 		dev_err(dev, "Failed to create sysfs attributes\n");
 		goto destroy_lpc_mec;
 	}
+
+	/*
+	 * 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);
 destroy_lpc_mec:
 	cros_ec_lpc_mec_destroy();
 	return ret;
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..d8ed791da82d
--- /dev/null
+++ b/drivers/rtc/rtc-wilco-ec.c
@@ -0,0 +1,178 @@
+// 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 DRV_NAME "rtc-wilco-ec"
+
+#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),
+	};
+	struct rtc_time calc_tm;
+	unsigned long time;
+	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;
+	/*
+	 * The RTC stores the month value as 1-12 but the kernel expects 0-11,
+	 * so ensure invalid/zero month value from RTC is not converted to -1.
+	 */
+	tm->tm_mon	= rtc.month ? rtc.month - 1 : 0;
+	tm->tm_year	= rtc.year + (rtc.century * 100) - 1900;
+	tm->tm_yday	= rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year);
+
+	/* Compute day of week */
+	rtc_tm_to_time(tm, &time);
+	rtc_time_to_tm(time, &calc_tm);
+	tm->tm_wday = calc_tm.tm_wday;
+
+	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 */
+	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 = devm_rtc_device_register(&pdev->dev,
+							  DRV_NAME,
+							  &wilco_ec_rtc_ops,
+							  THIS_MODULE);
+	if (IS_ERR(rtc))
+		return PTR_ERR(rtc);
+
+	return 0;
+}
+
+static struct platform_driver wilco_ec_rtc_driver = {
+	.driver = {
+		.name = DRV_NAME,
+	},
+	.probe = wilco_ec_rtc_probe,
+};
+
+module_platform_driver(wilco_ec_rtc_driver);
+
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_AUTHOR("Nick Crews <ncrews@google.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Wilco EC RTC driver");
-- 
2.20.1.97.g81188d93c3-goog


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

* [PATCH v2 6/9] platform/chrome: Add event handling
  2019-01-14 22:03 [PATCH v2 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
                   ` (4 preceding siblings ...)
  2019-01-14 22:03 ` [PATCH v2 5/9] platform/chrome: rtc: Add RTC driver for Wilco EC Nick Crews
@ 2019-01-14 22:03 ` Nick Crews
  2019-01-14 22:03 ` [PATCH v2 7/9] platform/chrome: Add EC properties Nick Crews
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Nick Crews @ 2019-01-14 22:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: groeck, sjg, dlaurie, Duncan Laurie, Nick Crews,
	Enric Balletbo i Serra, Benson Leung

From: Duncan Laurie <dlaurie@google.com>

The Wilco Embedded Controller 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.

> evtest /dev/input/event6
Input driver version is 1.0.1
Input device ID: bus 0x19 vendor 0x0 product 0x0 version 0x0
Input device name: "Wilco EC hotkeys"
Supported events:
  Event type 0 (EV_SYN)
  Event type 1 (EV_KEY)
    Event code 224 (KEY_BRIGHTNESSDOWN)
    Event code 225 (KEY_BRIGHTNESSUP)
    Event code 240 (KEY_UNKNOWN)
  Event type 4 (EV_MSC)
    Event code 4 (MSC_SCAN)
Properties:
Testing ... (interrupt to exit)
Event: type 4 (EV_MSC), code 4 (MSC_SCAN), value 57
Event: type 1 (EV_KEY), code 224 (KEY_BRIGHTNESSDOWN), value 1
Event: -------------- SYN_REPORT ------------
Event: type 1 (EV_KEY), code 224 (KEY_BRIGHTNESSDOWN), value 0
Event: -------------- SYN_REPORT ------------
Event: type 4 (EV_MSC), code 4 (MSC_SCAN), value 58
Event: type 1 (EV_KEY), code 225 (KEY_BRIGHTNESSUP), value 1
Event: -------------- SYN_REPORT ------------
Event: type 1 (EV_KEY), code 225 (KEY_BRIGHTNESSUP), value 0
Event: -------------- SYN_REPORT ------------

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

Changes in v2:
- rm "wilco_ec_event -" prefix from docstring
- rm license boiler plate
- Add sysfs directory documentation
- Fix cosmetics
- Remove duplicate error messages

 drivers/platform/chrome/wilco_ec/Makefile |   3 +-
 drivers/platform/chrome/wilco_ec/core.c   |  14 +-
 drivers/platform/chrome/wilco_ec/event.c  | 347 ++++++++++++++++++++++
 include/linux/platform_data/wilco-ec.h    |  34 +++
 4 files changed, 396 insertions(+), 2 deletions(-)
 create mode 100644 drivers/platform/chrome/wilco_ec/event.c

diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
index 7965a39e2e7a..57ef68ac1dd7 100644
--- a/drivers/platform/chrome/wilco_ec/Makefile
+++ b/drivers/platform/chrome/wilco_ec/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 
-wilco_ec-objs				:= core.o mailbox.o sysfs.o legacy.o
+wilco_ec-objs				:= core.o mailbox.o sysfs.o legacy.o \
+					   event.o
 obj-$(CONFIG_WILCO_EC)			+= wilco_ec.o
diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
index 61dacd582582..1acc566d3f9d 100644
--- a/drivers/platform/chrome/wilco_ec/core.c
+++ b/drivers/platform/chrome/wilco_ec/core.c
@@ -72,6 +72,13 @@ static int wilco_ec_probe(struct platform_device *pdev)
 		goto destroy_lpc_mec;
 	}
 
+	/* Prepare to handle events */
+	ret = wilco_ec_event_init(ec);
+	if (ret < 0) {
+		dev_err(dev, "Failed to setup event handling\n");
+		goto remove_sysfs;
+	}
+
 	/*
 	 * Register a RTC platform device that will get picked up by the RTC
 	 * subsystem. This platform device will get freed when its parent device
@@ -83,11 +90,13 @@ static int wilco_ec_probe(struct platform_device *pdev)
 	if (IS_ERR(child_pdev)) {
 		dev_err(dev, "Failed to create RTC platform device\n");
 		ret = PTR_ERR(child_pdev);
-		goto remove_sysfs;
+		goto remove_events;
 	}
 
 	return 0;
 
+remove_events:
+	wilco_ec_event_remove(ec);
 remove_sysfs:
 	wilco_ec_sysfs_remove(ec);
 destroy_lpc_mec:
@@ -99,6 +108,9 @@ static int wilco_ec_remove(struct platform_device *pdev)
 {
 	struct wilco_ec_device *ec = platform_get_drvdata(pdev);
 
+	/* Stop handling EC events */
+	wilco_ec_event_remove(ec);
+
 	/* Remove sysfs attributes */
 	wilco_ec_sysfs_remove(ec);
 
diff --git a/drivers/platform/chrome/wilco_ec/event.c b/drivers/platform/chrome/wilco_ec/event.c
new file mode 100644
index 000000000000..7601c02dffe8
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/event.c
@@ -0,0 +1,347 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Event handling for Wilco Embedded Controller
+ *
+ * Copyright 2018 Google LLC
+ *
+ * The Wilco Embedded Controller 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.
+ */
+
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <linux/list.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/platform_device.h>
+
+/* ACPI Notify event code indicating event data is available */
+#define EC_ACPI_NOTIFY_EVENT		0x90
+
+/* ACPI Method to execute to retrieve event data buffer from the EC */
+#define EC_ACPI_GET_EVENT		"^QSET"
+
+/* Maximum number of words in event data returned by the EC */
+#define EC_ACPI_MAX_EVENT_DATA		6
+
+/* Keep at most 100 events in the queue */
+#define EC_EVENT_QUEUE_MAX		100
+
+/**
+ * enum ec_event_type - EC event categories.
+ * @EC_EVENT_TYPE_HOTKEY: Hotkey event for handling special keys.
+ * @EC_EVENT_TYPE_NOTIFY: EC feature state changes.
+ * @EC_EVENT_TYPE_SYSTEM: EC system messages.
+ */
+enum ec_event_type {
+	EC_EVENT_TYPE_HOTKEY = 0x10,
+	EC_EVENT_TYPE_NOTIFY = 0x11,
+	EC_EVENT_TYPE_SYSTEM = 0x12,
+};
+
+/**
+ * struct ec_event - Extended event returned by the EC.
+ * @size: Number of words in structure after the size word.
+ * @type: Extended event type from &enum ec_event_type.
+ * @event: Event data words.  Max count is %EC_ACPI_MAX_EVENT_DATA.
+ */
+struct ec_event {
+	u16 size;
+	u16 type;
+	u16 event[0];
+} __packed;
+
+/**
+ * struct ec_event_entry - Event queue entry.
+ * @list: List node.
+ * @size: Number of bytes in event structure.
+ * @event: Extended event returned by the EC.  This should be the last
+ *         element because &struct ec_event includes a zero length array.
+ */
+struct ec_event_entry {
+	struct list_head list;
+	size_t size;
+	struct ec_event event;
+};
+
+static const struct key_entry wilco_ec_keymap[] = {
+	{ KE_KEY, 0x0057, { KEY_BRIGHTNESSDOWN } },
+	{ KE_KEY, 0x0058, { KEY_BRIGHTNESSUP } },
+	{ KE_END, 0 }
+};
+
+/**
+ * wilco_ec_handle_events() - Handle Embedded Controller events.
+ * @ec: EC device.
+ * @buf: Buffer of event data.
+ * @length: Length of event data buffer.
+ *
+ * Return: Number of events in queue or negative error code on failure.
+ *
+ * This function will handle EC extended events, sending hotkey events
+ * to the input subsystem and queueing others to be read by userspace.
+ */
+static int wilco_ec_handle_events(struct wilco_ec_device *ec,
+				  u8 *buf, u32 length)
+{
+	struct wilco_ec_event *queue = &ec->event;
+	struct ec_event *event;
+	struct ec_event_entry *entry;
+	size_t entry_size, num_words;
+	u32 offset = 0;
+
+	while (offset < length) {
+		event = (struct ec_event *)(buf + offset);
+		if (!event)
+			return -EINVAL;
+
+		dev_dbg(ec->dev, "EC event type 0x%02x size %d\n", event->type,
+			event->size);
+
+		/* Number of 16bit event data words is size - 1 */
+		num_words = event->size - 1;
+		entry_size = sizeof(*event) + (num_words * sizeof(u16));
+
+		if (num_words > EC_ACPI_MAX_EVENT_DATA) {
+			dev_err(ec->dev, "Invalid event word count: %d > %d\n",
+				num_words, EC_ACPI_MAX_EVENT_DATA);
+			return -EOVERFLOW;
+		};
+
+		/* Ensure event does not overflow the available buffer */
+		if ((offset + entry_size) > length) {
+			dev_err(ec->dev, "Event exceeds buffer: %d > %d\n",
+				offset + entry_size, length);
+			return -EOVERFLOW;
+		}
+
+		/* Point to the next event in the buffer */
+		offset += entry_size;
+
+		/* Hotkeys are sent to the input subsystem */
+		if (event->type == EC_EVENT_TYPE_HOTKEY) {
+			if (sparse_keymap_report_event(queue->input,
+						       event->event[0],
+						       1, true))
+				continue;
+
+			/* Unknown hotkeys are put into the event queue */
+			dev_dbg(ec->dev, "Unknown hotkey 0x%04x\n",
+				event->event[0]);
+		}
+
+		/* Prepare event for the queue */
+		entry = kzalloc(entry_size, GFP_KERNEL);
+		if (!entry)
+			return -ENOMEM;
+
+		/* Copy event data */
+		entry->size = entry_size;
+		memcpy(&entry->event, event, entry->size);
+
+		/* Add this event to the queue */
+		mutex_lock(&queue->lock);
+		list_add_tail(&entry->list, &queue->list);
+		queue->count++;
+		mutex_unlock(&queue->lock);
+	}
+
+	return queue->count;
+}
+
+/**
+ * wilco_ec_acpi_notify() - Handler called by ACPI subsystem for Notify.
+ * @device: EC ACPI device.
+ * @value: Value passed to Notify() in ACPI.
+ * @data: Private data, pointer to EC device.
+ */
+static void wilco_ec_acpi_notify(acpi_handle device, u32 value, void *data)
+{
+	struct wilco_ec_device *ec = data;
+	struct acpi_buffer event_buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object *obj;
+	acpi_status status;
+	int count;
+
+	/* Currently only handle event notifications */
+	if (value != EC_ACPI_NOTIFY_EVENT) {
+		dev_err(ec->dev, "Invalid event: 0x%08x\n", value);
+		return;
+	}
+
+	/* Execute ACPI method to get event data buffer */
+	status = acpi_evaluate_object(device, EC_ACPI_GET_EVENT,
+				      NULL, &event_buffer);
+	if (ACPI_FAILURE(status)) {
+		dev_err(ec->dev, "Error executing ACPI method %s()\n",
+			 EC_ACPI_GET_EVENT);
+		return;
+	}
+
+	obj = (union acpi_object *)event_buffer.pointer;
+	if (!obj) {
+		dev_err(ec->dev, "Nothing returned from %s()\n",
+			EC_ACPI_GET_EVENT);
+		return;
+	}
+	if (obj->type != ACPI_TYPE_BUFFER) {
+		dev_err(ec->dev, "Invalid object returned from %s()\n",
+			EC_ACPI_GET_EVENT);
+		kfree(obj);
+		return;
+	}
+	if (obj->buffer.length < sizeof(struct ec_event)) {
+		dev_err(ec->dev, "Invalid buffer length %d from %s()\n",
+			obj->buffer.length, EC_ACPI_GET_EVENT);
+		kfree(obj);
+		return;
+	}
+
+	/* Handle events and notify sysfs if any queued for userspace */
+	count = wilco_ec_handle_events(ec, obj->buffer.pointer,
+				       obj->buffer.length);
+
+	if (count > 0) {
+		dev_dbg(ec->dev, "EC event queue has %d entries\n", count);
+		sysfs_notify(&ec->dev->kobj, NULL, "event");
+	}
+
+	kfree(obj);
+}
+
+static ssize_t wilco_ec_event_read(struct file *filp, struct kobject *kobj,
+				   struct bin_attribute *attr,
+				   char *buf, loff_t off, size_t count)
+{
+	struct wilco_ec_device *ec = attr->private;
+	struct ec_event_entry *entry;
+
+	/* Only supports reading full events */
+	if (off != 0)
+		return -EINVAL;
+
+	/* Remove the first event and provide it to userspace */
+	mutex_lock(&ec->event.lock);
+	entry = list_first_entry_or_null(&ec->event.list,
+					 struct ec_event_entry, list);
+	if (entry) {
+		if (entry->size < count)
+			count = entry->size;
+		memcpy(buf, &entry->event, count);
+		list_del(&entry->list);
+		kfree(entry);
+		ec->event.count--;
+	} else {
+		count = 0;
+	}
+	mutex_unlock(&ec->event.lock);
+
+	return count;
+}
+
+static void wilco_ec_event_clear(struct wilco_ec_device *ec)
+{
+	struct ec_event_entry *entry, *next;
+
+	mutex_lock(&ec->event.lock);
+
+	/* Clear the event queue */
+	list_for_each_entry_safe(entry, next, &ec->event.list, list) {
+		list_del(&entry->list);
+		kfree(entry);
+		ec->event.count--;
+	}
+
+	mutex_unlock(&ec->event.lock);
+}
+
+int wilco_ec_event_init(struct wilco_ec_device *ec)
+{
+	struct wilco_ec_event *event = &ec->event;
+	struct device *dev = ec->dev;
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	acpi_status status;
+	int ret;
+
+	if (!adev) {
+		dev_err(dev, "Unable to find Wilco ACPI Device\n");
+		return -ENODEV;
+	}
+
+	INIT_LIST_HEAD(&event->list);
+	mutex_init(&event->lock);
+
+	/* Allocate input device for hotkeys */
+	event->input = input_allocate_device();
+	if (!event->input)
+		return -ENOMEM;
+	event->input->name = "Wilco EC hotkeys";
+	event->input->phys = "ec/input0";
+	event->input->id.bustype = BUS_HOST;
+	ret = sparse_keymap_setup(event->input, wilco_ec_keymap, NULL);
+	if (ret) {
+		dev_err(dev, "Unable to setup input device keymap\n");
+		input_free_device(event->input);
+		return ret;
+	}
+	ret = input_register_device(event->input);
+	if (ret) {
+		dev_err(dev, "Unable to register input device\n");
+		input_free_device(event->input);
+		return ret;
+	}
+
+	/* Create sysfs attribute for userspace event handling */
+	sysfs_bin_attr_init(&event->attr);
+	event->attr.attr.name = "event";
+	event->attr.attr.mode = 0400;
+	event->attr.read = wilco_ec_event_read;
+	event->attr.private = ec;
+	ret = device_create_bin_file(dev, &event->attr);
+	if (ret) {
+		dev_err(dev, "Failed to create event attribute in sysfs\n");
+		input_unregister_device(event->input);
+		return ret;
+	}
+
+	/* Install ACPI handler for Notify events */
+	status = acpi_install_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
+					     wilco_ec_acpi_notify, ec);
+	if (ACPI_FAILURE(status)) {
+		dev_err(dev, "Failed to register notifier %08x\n", status);
+		device_remove_bin_file(dev, &event->attr);
+		input_unregister_device(event->input);
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+void wilco_ec_event_remove(struct wilco_ec_device *ec)
+{
+	struct acpi_device *adev = ACPI_COMPANION(ec->dev);
+
+	/* Stop new events */
+	if (adev)
+		acpi_remove_notify_handler(adev->handle, ACPI_ALL_NOTIFY,
+					   wilco_ec_acpi_notify);
+
+	/* Remove event interfaces */
+	device_remove_bin_file(ec->dev, &ec->event.attr);
+	input_unregister_device(ec->event.input);
+
+	/* Clear the event queue */
+	wilco_ec_event_clear(ec);
+}
diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
index 7c6ab6de7239..d6bfc114c50e 100644
--- a/include/linux/platform_data/wilco-ec.h
+++ b/include/linux/platform_data/wilco-ec.h
@@ -9,7 +9,9 @@
 #define WILCO_EC_H
 
 #include <linux/device.h>
+#include <linux/input.h>
 #include <linux/kernel.h>
+#include <linux/list.h>
 
 #define WILCO_EC_FLAG_NO_RESPONSE	BIT(0) /* EC does not respond */
 #define WILCO_EC_FLAG_EXTENDED_DATA	BIT(1) /* EC returns 256 data bytes */
@@ -24,6 +26,22 @@
 /* Extended commands have 256 bytes of response data */
 #define EC_MAILBOX_DATA_SIZE_EXTENDED	256
 
+/**
+ * struct wilco_ec_event - EC extended events.
+ * @lock: Mutex to guard the list of events.
+ * @list: Queue of EC events to be provided to userspace.
+ * @attr: Sysfs attribute for userspace to read events.
+ * @count: Count of events in the queue.
+ * @input: Input device for hotkey events.
+ */
+struct wilco_ec_event {
+	struct mutex lock;
+	struct list_head list;
+	struct bin_attribute attr;
+	size_t count;
+	struct input_dev *input;
+};
+
 /**
  * struct wilco_ec_device - Wilco Embedded Controller handle.
  * @dev: Device handle.
@@ -34,6 +52,7 @@
  * @data_buffer: Buffer used for EC communication.  The same buffer
  *               is used to hold the request and the response.
  * @data_size: Size of the data buffer used for EC communication.
+ * @event: EC extended event handler.
  */
 struct wilco_ec_device {
 	struct device *dev;
@@ -43,6 +62,7 @@ struct wilco_ec_device {
 	struct resource *io_packet;
 	void *data_buffer;
 	size_t data_size;
+	struct wilco_ec_event event;
 };
 
 /**
@@ -149,4 +169,18 @@ int wilco_ec_sysfs_init(struct wilco_ec_device *ec);
  */
 void wilco_ec_sysfs_remove(struct wilco_ec_device *ec);
 
+/**
+ * wilco_ec_event_init() - Prepare to handle EC events.
+ * @ec: EC device.
+ *
+ * Return: 0 for success or negative error code on failure.
+ */
+int wilco_ec_event_init(struct wilco_ec_device *ec);
+
+/**
+ * wilco_ec_event_remove() - Remove EC event handler.
+ * @ec: EC device.
+ */
+void wilco_ec_event_remove(struct wilco_ec_device *ec);
+
 #endif /* WILCO_EC_H */
-- 
2.20.1.97.g81188d93c3-goog


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

* [PATCH v2 7/9] platform/chrome: Add EC properties
  2019-01-14 22:03 [PATCH v2 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
                   ` (5 preceding siblings ...)
  2019-01-14 22:03 ` [PATCH v2 6/9] platform/chrome: Add event handling Nick Crews
@ 2019-01-14 22:03 ` Nick Crews
  2019-01-14 22:03 ` [PATCH v2 8/9] platform/chrome: Add peakshift and adv_batt_charging Nick Crews
  2019-01-14 22:03 ` [PATCH v2 9/9] platform/chrome: Add binary telemetry attributes Nick Crews
  8 siblings, 0 replies; 17+ messages in thread
From: Nick Crews @ 2019-01-14 22:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: groeck, sjg, dlaurie, Nick Crews, Duncan Laurie,
	Enric Balletbo i Serra, Benson Leung

A Property is typically a data item that is stored to NVRAM.
Each of these data items has an index associated with it
known as the Property ID (PID). The Property ID is
used by the system BIOS (and EC) to refer to the Property.
Properties may have variable lengths. Many features are
implemented primarily by EC Firmware with system BIOS
just supporting user configuration via BIOS SETUP and/or
SMBIOS changes. In order to implement many of these types of
features the user configuration information is saved to and
retrieved from the EC. The EC stores this configuration
information to NVRAM and then can use it while the system
BIOS is not running or during early boot. Although this
is a typical scenario there may be other reasons to store
information in the EC NVRAM instead of the System NVRAM.
Most of the property services do not have a valid failure
condition, so this field can be ignored. For items that
are write once, a failure is returned when a second
write is attempted.

Add a get and set interface for EC properties.
properties live within the "properties" directory.
Most of the added properties are boolean, but this also
provides the interface for non-boolean properties,
which will be used late for scheduling power routines.

The wilco_ec_sysfs_util.h stuff will be used for
future attributes as well.

> cd /sys/bus/platform/devices/GOOG000C\:00/
> echo 0 > properties/global_mic_mute_led
[mic mute led on keyboard turns off]
> cat
0
> echo 1 > properties/global_mic_mute_led
[mic mute led on keyboard turns on]
> cat properties/global_mic_mute_led
1
> cat properties/wireless_sw_wlan
cat: wireless_sw_wlan: Permission denied
[Good, that is supposed to be write-only]
> echo 0 > properties/wireless_sw_wlan

Signed-off-by: Nick Crews <ncrews@google.com>
---

Changes in v2:
- rm license boiler plate
- rm "wilco_ec_properties -" prefix from docstring
- Add documentation

 .../ABI/testing/sysfs-platform-wilcoec        |  48 +++
 drivers/platform/chrome/wilco_ec/Makefile     |   2 +-
 drivers/platform/chrome/wilco_ec/properties.c | 344 ++++++++++++++++++
 drivers/platform/chrome/wilco_ec/properties.h | 179 +++++++++
 drivers/platform/chrome/wilco_ec/sysfs.c      |  55 ++-
 drivers/platform/chrome/wilco_ec/util.h       |  38 ++
 6 files changed, 664 insertions(+), 2 deletions(-)
 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/util.h

diff --git a/Documentation/ABI/testing/sysfs-platform-wilcoec b/Documentation/ABI/testing/sysfs-platform-wilcoec
index cac2cf11835f..b09e2473f948 100644
--- a/Documentation/ABI/testing/sysfs-platform-wilcoec
+++ b/Documentation/ABI/testing/sysfs-platform-wilcoec
@@ -45,3 +45,51 @@ Description:
 		// included after the raw hex.
 		$ cat raw
 		00 31 32 2f 32 31 2f 31 38 00 38 00 01 00 2f 00  .12/21/18.8...
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/properties/global_mic_mute_led
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Read/write the microphone mute led on the keyboard on and off.
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/properties/fn_lock
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Fn Toggle - main feature enable/disable setting.
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/properties/nic
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Enable NIC (network interface card) on the dock.
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/properties/ext_usb_port_en
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		External USB port enable.
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/wireless_sw_wlan
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Wireless switch - WLAN.
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/properties/auto_boot_on_trinity_dock_attach
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Enable wake when dock is attached to the system.
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/properties/ich_azalia_en
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Enable Audio in the Dock.
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/properties/sign_of_life_kbbl
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Enable early Keyboard BackLight at boot.
diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
index 57ef68ac1dd7..9c9e67171652 100644
--- a/drivers/platform/chrome/wilco_ec/Makefile
+++ b/drivers/platform/chrome/wilco_ec/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 
 wilco_ec-objs				:= core.o mailbox.o sysfs.o legacy.o \
-					   event.o
+					   event.o properties.o
 obj-$(CONFIG_WILCO_EC)			+= wilco_ec.o
diff --git a/drivers/platform/chrome/wilco_ec/properties.c b/drivers/platform/chrome/wilco_ec/properties.c
new file mode 100644
index 000000000000..e9d096c7d632
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/properties.c
@@ -0,0 +1,344 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Add ability to set/get properties of Wilco Embedded Controller
+ *
+ * Copyright 2018 Google LLC
+ *
+ * A Property is typically a data item that is stored to NVRAM.
+ * Each of these data items has an index associated with it
+ * known as the Property ID (PID). The Property ID is
+ * used by the system BIOS (and EC) to refer to the Property.
+ * Properties may have variable lengths. Many features are
+ * implemented primarily by EC Firmware with system BIOS
+ * just supporting user configuration via BIOS SETUP and/or
+ * SMBIOS changes. In order to implement many of these types of
+ * features the user configuration information is saved to and
+ * retrieved from the EC. The EC stores this configuration
+ * information to NVRAM and then can use it while the system
+ * BIOS is not running or during early boot. Although this
+ * is a typical scenario there may be other reasons to store
+ * information in the EC NVRAM instead of the System NVRAM.
+ * Most of the property services do not have a valid failure
+ * condition, so this field can be ignored. For items that
+ * are write once, a failure is returned when a second
+ * write is attempted.
+ *
+ * Add a get and set interface for EC properties.
+ * properties live within the "properties" directory.
+ * Most of the added properties are boolean, but this also
+ * provides the interface for non-boolean properties,
+ * which will be used late for scheduling power routines.
+ */
+
+#include <linux/device.h>
+#include <linux/platform_data/wilco-ec.h>
+
+#include "properties.h"
+#include "util.h"
+
+/* Payload length for get/set properties */
+#define PROPERTY_DATA_MAX_LENGTH 4
+
+struct ec_property_get_request {
+	u32 property_id;
+	u8 length;
+} __packed;
+
+struct ec_property_set_request {
+	u32 property_id;
+	u8 length;
+	u8 data[PROPERTY_DATA_MAX_LENGTH];
+} __packed;
+
+struct ec_property_response {
+	u8 status;
+	u8 sub_function;
+	u32 property_id;
+	u8 length;
+	u8 data[PROPERTY_DATA_MAX_LENGTH];
+} __packed;
+
+/* Store a 32 bit property ID into an array or a field in a struct, LSB first */
+static inline void fill_property_id(u32 property_id, u8 field[])
+{
+	field[0] =  property_id        & 0xff;
+	field[1] = (property_id >> 8)  & 0xff;
+	field[2] = (property_id >> 16) & 0xff;
+	field[3] = (property_id >> 24) & 0xff;
+}
+
+/* Extract 32 bit property ID from an array or a field in a struct, LSB first */
+static inline u32 extract_property_id(u8 field[])
+{
+	return (uint32_t)field[0]	|
+	       (uint32_t)field[1] << 8  |
+	       (uint32_t)field[2] << 16 |
+	       (uint32_t)field[3] << 24;
+}
+
+/**
+ * check_property_response() - Verify that the response from the EC is valid.
+ * @ec: EC device
+ * @rs: bytes sent back from the EC, filled into struct
+ * @op: Which of [SET, GET, SYNC] we are responding to
+ * @expected_property_id: Property ID that we were trying to read
+ * @expected_length: Number of bytes of actual payload we expected
+ * @expected_data: What we expect the EC to echo back for a SET. For GETting
+ *		   or SYNCing, we don't know the response, so use NULL to ignore
+ *
+ * Return: 0 on success, -EBADMSG on failure.
+ */
+static int check_property_response(struct wilco_ec_device *ec,
+				   struct ec_property_response *rs,
+				   enum get_set_sync_op op,
+				   u32 expected_property_id, u8 expected_length,
+				   const u8 expected_data[])
+{
+	u32 received_property_id;
+	int i;
+
+	/* check for success/failure flag */
+	if (rs->status) {
+		dev_err(ec->dev, "EC reports failure to get property");
+		return -EBADMSG;
+	}
+
+	/* Which subcommand is the EC responding to? */
+	if (rs->sub_function != op) {
+		dev_err(ec->dev, "For SET/GET/SYNC, EC replied %d, expected %d",
+			rs->sub_function, op);
+		return -EBADMSG;
+	}
+
+	/* Check that returned property_id is what we expect */
+	received_property_id = extract_property_id((u8 *)&rs->property_id);
+	if (received_property_id != expected_property_id) {
+		dev_err(ec->dev,
+			"EC responded to property_id 0x%08x, expected 0x%08x",
+			received_property_id, expected_property_id);
+		return -EBADMSG;
+	}
+
+	/* Did we get the correct number of bytes as a payload? */
+	if (rs->length != expected_length) {
+		dev_err(ec->dev, "EC returned %d bytes when we expected %d",
+			rs->length, expected_length);
+		return -EBADMSG;
+	}
+
+	/* Check that the actual data returned was what we expected */
+	if (expected_length < 1 || !expected_data)
+		return 0;
+	for (i = 0; i < expected_length; i++) {
+		if (rs->data[i] != expected_data[i]) {
+			dev_err(ec->dev, "returned[%d]=%2x != expected[%d]=%2x",
+				i, rs->data[i], i, expected_data[i]);
+			return -EBADMSG;
+		}
+	}
+
+	return 0;
+}
+
+static inline int check_get_property_response(struct wilco_ec_device *ec,
+					      struct ec_property_response *rs,
+					      u32 expected_property_id,
+					      u8 expected_length)
+{
+	return check_property_response(ec, rs, OP_GET, expected_property_id,
+				       expected_length, NULL);
+}
+
+static inline int check_set_property_response(struct wilco_ec_device *ec,
+					      struct ec_property_response *rs,
+					      enum get_set_sync_op op,
+					      u32 expected_property_id,
+					      u8 expected_length,
+					      const u8 expected_data[])
+{
+	return check_property_response(ec, rs, op, expected_property_id,
+				       expected_length, expected_data);
+}
+
+ssize_t wilco_ec_get_property(struct wilco_ec_device *ec, u32 property_id,
+			      u8 result_length, u8 *result)
+{
+	int ret, response_valid;
+	struct ec_property_get_request rq;
+	struct ec_property_response rs;
+	struct wilco_ec_message msg = {
+		.type = WILCO_EC_MSG_PROPERTY,
+		.flags = WILCO_EC_FLAG_RAW,
+		.command = OP_GET,
+		.request_data = &rq,
+		.request_size = sizeof(rq),
+		.response_data = &rs,
+		.response_size = sizeof(rs),
+	};
+
+	/* Create the request struct */
+	if (result_length < 1) {
+		dev_err(ec->dev,
+			"Requested %d bytes when getting property, min is 0\n",
+			result_length);
+		return -EINVAL;
+	}
+	if (result_length > PROPERTY_DATA_MAX_LENGTH) {
+		dev_err(ec->dev,
+			"Requested %d bytes when getting property, max is %d\n",
+			result_length, PROPERTY_DATA_MAX_LENGTH);
+		return -EINVAL;
+	}
+	fill_property_id(property_id, (u8 *)&(rq.property_id));
+	rq.length = 0;
+
+	/* send and receive */
+	ret = wilco_ec_mailbox(ec, &msg);
+	if (ret < 0) {
+		dev_err(ec->dev, "Get Property 0x%08x command failed\n",
+			property_id);
+		return ret;
+	}
+
+	/* verify that the response was valid */
+	response_valid = check_get_property_response(ec, &rs, property_id,
+						     result_length);
+	if (response_valid < 0)
+		return response_valid;
+
+	memcpy(result, &rs.data, result_length);
+	return ret;
+}
+
+ssize_t wilco_ec_set_property(struct wilco_ec_device *ec,
+			      enum get_set_sync_op op, u32 property_id,
+			      u8 length, const u8 *data)
+{
+	int ret;
+	struct ec_property_set_request rq;
+	struct ec_property_response rs;
+	u8 request_length = sizeof(rq) - PROPERTY_DATA_MAX_LENGTH + length;
+	struct wilco_ec_message msg = {
+		.type = WILCO_EC_MSG_PROPERTY,
+		.flags = WILCO_EC_FLAG_RAW,
+		.command = op,
+		.request_data = &rq,
+		.request_size = request_length,
+		.response_data = &rs,
+		.response_size = sizeof(rs),
+	};
+
+	/* make request */
+	if (op != OP_SET && op != OP_SYNC) {
+		dev_err(ec->dev, "Set op must be OP_SET | OP_SYNC, got %d", op);
+		return -EINVAL;
+	}
+	if (length < 1) {
+		dev_err(ec->dev,
+			"Sending %d bytes when setting property, min is 1",
+			length);
+		return -EINVAL;
+	}
+	if (length > PROPERTY_DATA_MAX_LENGTH) {
+		dev_err(ec->dev,
+			"Sending %d bytes when setting property, max is %d",
+			length, PROPERTY_DATA_MAX_LENGTH);
+		return -EINVAL;
+	}
+	fill_property_id(property_id, (u8 *)&(rq.property_id));
+	rq.length = length;
+	memcpy(rq.data, data, length);
+
+	/* send and receive */
+	ret = wilco_ec_mailbox(ec, &msg);
+	if (ret < 0) {
+		dev_err(ec->dev, "Set Property 0x%08x command failed\n",
+			property_id);
+		return ret;
+	}
+
+	/* verify that the response was valid, EC echoing back stored value */
+	ret = check_set_property_response(ec, &rs, op, property_id,
+						     length, data);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+ssize_t wilco_ec_get_bool_prop(struct device *dev, u32 property_id,
+			       char *result)
+{
+	struct wilco_ec_device *ec = dev_get_drvdata(dev);
+	int ret;
+
+	ret = wilco_ec_get_property(ec, property_id, 1, result);
+	if (ret < 0)
+		return ret;
+
+	/* convert the raw byte response into ascii */
+	switch (result[0]) {
+	case 0:
+		result[0] = '0';
+		break;
+	case 1:
+		result[0] = '1';
+		break;
+	default:
+		dev_err(ec->dev, "Expected 0 or 1 as response, got %02x",
+			result[0]);
+		return -EBADMSG;
+	}
+
+	/* Tack on a newline */
+	result[1] = '\n';
+	return 2;
+}
+
+ssize_t wilco_ec_set_bool_prop(struct device *dev, enum get_set_sync_op op,
+			       u32 property_id, const char *buf, size_t count)
+{
+	struct wilco_ec_device *ec = dev_get_drvdata(dev);
+	bool enable;
+	u8 param;
+	int ret;
+
+	ret = kstrtobool(buf, &enable);
+	if (ret) {
+		dev_err(dev, "Unable to parse '%s' to a bool", buf);
+		return ret;
+	}
+	param = enable;
+
+	ret = wilco_ec_set_property(ec, op, property_id, 1, &param);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+ssize_t wilco_ec_bool_prop_show(struct kobject *kobj,
+				struct kobj_attribute *attr, char *buf)
+{
+	struct property_attribute *prop_attr;
+	struct device *dev;
+
+	prop_attr = container_of(attr, struct property_attribute, kobj_attr);
+	dev = device_from_kobject(kobj);
+
+	return wilco_ec_get_bool_prop(dev, prop_attr->pid, buf);
+}
+
+ssize_t wilco_ec_bool_prop_store(struct kobject *kobj,
+				 struct kobj_attribute *attr, const char *buf,
+				 size_t count)
+{
+	struct property_attribute *prop_attr;
+	struct device *dev;
+
+	prop_attr = container_of(attr, struct property_attribute, kobj_attr);
+	dev = device_from_kobject(kobj);
+
+	return wilco_ec_set_bool_prop(dev, prop_attr->op, prop_attr->pid, buf,
+				      count);
+}
diff --git a/drivers/platform/chrome/wilco_ec/properties.h b/drivers/platform/chrome/wilco_ec/properties.h
new file mode 100644
index 000000000000..b95dbaf624f6
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/properties.h
@@ -0,0 +1,179 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Add ability to set/get properties of Wilco Embedded Controller
+ *
+ * Copyright 2018 Google LLC
+ *
+ * A Property is typically a data item that is stored to NVRAM.
+ * Each of these data items has an index associated with it
+ * known as the Property ID (PID). The Property ID is
+ * used by the system BIOS (and EC) to refer to the Property.
+ * Properties may have variable lengths. Many features are
+ * implemented primarily by EC Firmware with system BIOS
+ * just supporting user configuration via BIOS SETUP and/or
+ * SMBIOS changes. In order to implement many of these types of
+ * features the user configuration information is saved to and
+ * retrieved from the EC. The EC stores this configuration
+ * information to NVRAM and then can use it while the system
+ * BIOS is not running or during early boot. Although this
+ * is a typical scenario there may be other reasons to store
+ * information in the EC NVRAM instead of the System NVRAM.
+ * Most of the property services do not have a valid failure
+ * condition, so this field can be ignored. For items that
+ * are write once, a failure is returned when a second
+ * write is attempted.
+ *
+ * Add a get and set interface for EC properties.
+ * properties live within the "properties" directory.
+ * Most of the added properties are boolean, but this also
+ * provides the interface for non-boolean properties,
+ * which will be used late for scheduling power routines.
+ */
+
+#ifndef WILCO_EC_PROPERTIES_H
+#define WILCO_EC_PROPERTIES_H
+
+#include <linux/ctype.h>
+#include <linux/kobject.h>
+#include <linux/device.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/sysfs.h>
+
+#define PID_GLOBAL_MIC_MUTE_LED			0x0676
+#define PID_FN_LOCK				0x067b
+#define PID_NIC					0x04ea
+#define PID_EXT_USB_PORT_EN			0x0612
+#define PID_WIRELESS_SW_WLAN			0x0620
+#define PID_AUTO_BOOT_ON_TRINITY_DOCK_ATTACH	0x0725
+#define PID_ICH_AZALIA_EN			0x0a07
+#define PID_SIGN_OF_LIFE_KBBL			0x058f
+
+/**
+ * enum get_set_sync_op - three different subcommands for WILCO_EC_MSG_PROPERTY.
+ *
+ * OP_GET requests the property from the EC. OP_SET and OP_SYNC do the exact
+ * same thing from our perspective: save a property. Only one of them works for
+ * a given property, so each property uses either OP_GET and OP_SET, or
+ * OP_GET and OP_SYNC
+ */
+enum get_set_sync_op {
+	OP_GET = 0,
+	OP_SET = 1,
+	OP_SYNC = 4
+};
+
+/**
+ * struct property_attribute - A attribute representing an EC property
+ * @kobj_attr: The underlying kobj_attr that is registered with sysfs
+ * @pid: Property ID of this property
+ * @op: Either OP_SET or OP_SYNC, whichever this property uses
+ */
+struct property_attribute {
+	struct kobj_attribute kobj_attr;
+	u32 pid;
+	enum get_set_sync_op op;
+};
+
+/**
+ * wilco_ec_get_property() - Query a property from the EC
+ * @ec: EC device to query
+ * @property_id: Property ID
+ * @result_length: Number of bytes expected in result
+ * @result: Destination buffer for result, needs to be able to hold at least
+ *	    @result_length bytes
+ *
+ * Return: Number of bytes received from EC (AKA @result_length),
+ *	   negative error code on failure.
+ */
+ssize_t wilco_ec_get_property(struct wilco_ec_device *ec, u32 property_id,
+			      u8 result_length, u8 *result);
+
+/**
+ * wilco_ec_set_property() - Set a property on EC
+ * @ec: EC device to use
+ * @op: either OP_SET or OP_SYNC
+ * @property_id: Property ID
+ * @length: Number of bytes in input buffer @data
+ * @data: Input buffer
+ *
+ * Return: 0 on success, negative error code on failure
+ */
+ssize_t wilco_ec_set_property(struct wilco_ec_device *ec,
+			      enum get_set_sync_op op, u32 property_id,
+			      u8 length, const u8 *data);
+
+/**
+ * wilco_ec_get_bool_prop() - Get a boolean property from EC.
+ * @dev: EC device to use
+ * @property_id: Property ID
+ * @result: Destination buffer to be filled, needs to be able to hold at least
+ *	    two bytes. Will be filled with either "0\n" or "1\n" in ASCII
+ *
+ * Return: Number of bytes copied into result (AKA 2),
+ *	   or negative error code on failure.
+ */
+ssize_t wilco_ec_get_bool_prop(struct device *dev, u32 property_id,
+			       char *result);
+
+/**
+ * wilco_ec_set_bool_prop() - Set a boolean property on EC
+ * @dev: EC device to use
+ * @op: either OP_SET or OP_SYNC
+ * @property_id: Property ID
+ * @buf: Source buffer of ASCII string, parseable by kstrtobool()
+ * @count: Number of bytes in input buffer
+ *
+ * Return: Number of bytes consumed from input buffer (AKA @count),
+ *         or negative error code on failure.
+ */
+ssize_t wilco_ec_set_bool_prop(struct device *dev, enum get_set_sync_op op,
+			       u32 property_id, const char *buf, size_t count);
+
+/**
+ * wilco_ec_bool_prop_show() - Get a boolean property from the EC
+ * @kobj: Kobject representing the directory this attribute lives within
+ * @attr: Attribute stored within relevant "struct property_attribute"
+ * @buf: Destination buffer to be filled, needs to be able to hold at least
+ *	 two bytes. Will be filled with either "0\n" or "1\n" in ASCII
+ *
+ * Return: Number of bytes placed into output buffer (AKA 2),
+ *	   or negative error code on failure.
+ */
+ssize_t wilco_ec_bool_prop_show(struct kobject *kobj,
+				struct kobj_attribute *attr, char *buf);
+
+/**
+ * wilco_ec_bool_prop_store() - Store a boolean property on the EC
+ * @kobj: Kobject representing the directory this attribute lives within
+ * @attr: Attribute stored within relevant "struct property_attribute"
+ * @buf: Source buffer of ASCII string, parseable by kstrtobool()
+ * @count: Number of bytes in input buffer
+ *
+ * Return: Number bytes consumed from input buf (AKA @count),
+ *	   or negative error code on failure.
+ */
+ssize_t wilco_ec_bool_prop_store(struct kobject *kobj,
+				 struct kobj_attribute *attr, const char *buf,
+				 size_t count);
+
+#define BOOL_PROP_KOBJ_ATTR_RW(_name)					\
+__ATTR(_name, 0644, wilco_ec_bool_prop_show, wilco_ec_bool_prop_store)
+
+#define BOOL_PROP_KOBJ_ATTR_WO(_name)					\
+__ATTR(_name, 0200, NULL, wilco_ec_bool_prop_store)
+
+#define BOOLEAN_PROPERTY_RW_ATTRIBUTE(_op, _var, _name, _pid)	\
+struct property_attribute _var = {					\
+	.kobj_attr = BOOL_PROP_KOBJ_ATTR_RW(_name),			\
+	.pid = _pid,							\
+	.op = _op,							\
+}
+
+#define BOOLEAN_PROPERTY_WO_ATTRIBUTE(_op, _var, _name, _pid)	\
+struct property_attribute _var = {					\
+	.kobj_attr = BOOL_PROP_KOBJ_ATTR_WO(_name),			\
+	.pid = _pid,							\
+	.op = _op,							\
+}
+
+#endif /* WILCO_EC_PROPERTIES_H */
diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c b/drivers/platform/chrome/wilco_ec/sysfs.c
index 0611a73fcdce..b81f251f78fc 100644
--- a/drivers/platform/chrome/wilco_ec/sysfs.c
+++ b/drivers/platform/chrome/wilco_ec/sysfs.c
@@ -15,6 +15,7 @@
 #include <linux/sysfs.h>
 
 #include "legacy.h"
+#include "properties.h"
 
 #define WILCO_EC_ATTR_RO(_name)						\
 __ATTR(_name, 0444, wilco_ec_##_name##_show, NULL)
@@ -41,6 +42,38 @@ static struct attribute *wilco_ec_toplevel_attrs[] = {
 };
 ATTRIBUTE_GROUPS(wilco_ec_toplevel);
 
+/* Make property attributes, which will live inside GOOG000C:00/properties/  */
+BOOLEAN_PROPERTY_RW_ATTRIBUTE(OP_SET, bool_prop_attr_global_mic_mute_led,
+			      global_mic_mute_led, PID_GLOBAL_MIC_MUTE_LED);
+BOOLEAN_PROPERTY_RW_ATTRIBUTE(OP_SET, bool_prop_attr_fn_lock, fn_lock,
+			      PID_FN_LOCK);
+BOOLEAN_PROPERTY_RW_ATTRIBUTE(OP_SET, bool_prop_attr_nic, nic, PID_NIC);
+BOOLEAN_PROPERTY_RW_ATTRIBUTE(OP_SET, bool_prop_attr_ext_usb_port_en,
+			      ext_usb_port_en, PID_EXT_USB_PORT_EN);
+BOOLEAN_PROPERTY_WO_ATTRIBUTE(OP_SYNC, bool_prop_attr_wireless_sw_wlan,
+			      wireless_sw_wlan, PID_WIRELESS_SW_WLAN);
+BOOLEAN_PROPERTY_RW_ATTRIBUTE(OP_SET,
+			      bool_prop_attr_auto_boot_on_trinity_dock_attach,
+			      auto_boot_on_trinity_dock_attach,
+			      PID_AUTO_BOOT_ON_TRINITY_DOCK_ATTACH);
+BOOLEAN_PROPERTY_RW_ATTRIBUTE(OP_SET, bool_prop_attr_ich_azalia_en,
+			      ich_azalia_en, PID_ICH_AZALIA_EN);
+BOOLEAN_PROPERTY_RW_ATTRIBUTE(OP_SET, bool_prop_attr_sign_of_life_kbbl,
+			      sign_of_life_kbbl, PID_SIGN_OF_LIFE_KBBL);
+struct attribute *wilco_ec_property_attrs[] = {
+	&bool_prop_attr_global_mic_mute_led.kobj_attr.attr,
+	&bool_prop_attr_fn_lock.kobj_attr.attr,
+	&bool_prop_attr_nic.kobj_attr.attr,
+	&bool_prop_attr_ext_usb_port_en.kobj_attr.attr,
+	&bool_prop_attr_wireless_sw_wlan.kobj_attr.attr,
+	&bool_prop_attr_auto_boot_on_trinity_dock_attach.kobj_attr.attr,
+	&bool_prop_attr_ich_azalia_en.kobj_attr.attr,
+	&bool_prop_attr_sign_of_life_kbbl.kobj_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(wilco_ec_property);
+struct kobject *prop_dir_kobj;
+
 /**
  * wilco_ec_sysfs_init() - Initialize the sysfs directories and attributes
  * @dev: The device representing the EC
@@ -58,9 +91,27 @@ int wilco_ec_sysfs_init(struct wilco_ec_device *ec)
 	/* add the top-level attributes */
 	ret = sysfs_create_groups(&dev->kobj, wilco_ec_toplevel_groups);
 	if (ret)
-		return -ENOMEM;
+		goto err;
+
+	/* add the directory for properties */
+	prop_dir_kobj = kobject_create_and_add("properties", &dev->kobj);
+	if (!prop_dir_kobj)
+		goto rm_toplevel_attrs;
+
+	/* add the property attributes into the properties directory */
+	ret = sysfs_create_groups(prop_dir_kobj, wilco_ec_property_groups);
+	if (ret)
+		goto rm_properties_dir;
 
 	return 0;
+
+/* Go upwards through the directory structure, cleaning up */
+rm_properties_dir:
+	kobject_put(prop_dir_kobj);
+rm_toplevel_attrs:
+	sysfs_remove_groups(&dev->kobj, wilco_ec_toplevel_groups);
+err:
+	return -ENOMEM;
 }
 
 void wilco_ec_sysfs_remove(struct wilco_ec_device *ec)
@@ -68,5 +119,7 @@ void wilco_ec_sysfs_remove(struct wilco_ec_device *ec)
 	struct device *dev = ec->dev;
 
 	/* go upwards through the directory structure */
+	sysfs_remove_groups(prop_dir_kobj, wilco_ec_property_groups);
+	kobject_put(prop_dir_kobj);
 	sysfs_remove_groups(&dev->kobj, wilco_ec_toplevel_groups);
 }
diff --git a/drivers/platform/chrome/wilco_ec/util.h b/drivers/platform/chrome/wilco_ec/util.h
new file mode 100644
index 000000000000..708de8fd96b5
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/util.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * wilco_ec_sysfs_util - helpers for sysfs attributes of Wilco EC
+ *
+ * Copyright (C) 2018 Google LLC
+ */
+
+#ifndef WILCO_EC_SYSFS_UTIL_H
+#define WILCO_EC_SYSFS_UTIL_H
+
+#include <linux/kobject.h>
+#include <linux/device.h>
+#include <linux/string.h>
+
+/**
+ * device_from_kobject() - Get EC device from subdirectory's kobject.
+ * @kobj: kobject associated with a subdirectory
+ *
+ * When we place attributes within directories within the sysfs filesystem,
+ * at each callback we get a reference to the kobject representing the directory
+ * that that attribute is in. Somehow we need to get a pointer to the EC device.
+ * This goes up the directory structure a number of levels until reaching the
+ * top level for the EC device, and then finds the device from the root kobject.
+ *
+ * Example: for attribute GOOG000C:00/properties/peakshift/sunday,
+ * we would go up two levels, from peakshift to properties and then from
+ * properties to GOOG000C:00
+ *
+ * Return: a pointer to the device struct representing the EC.
+ */
+static inline struct device *device_from_kobject(struct kobject *kobj)
+{
+	while (strcmp(kobj->name, "GOOG000C:00") != 0)
+		kobj = kobj->parent;
+	return container_of(kobj, struct device, kobj);
+}
+
+#endif
-- 
2.20.1.97.g81188d93c3-goog


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

* [PATCH v2 8/9] platform/chrome: Add peakshift and adv_batt_charging
  2019-01-14 22:03 [PATCH v2 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
                   ` (6 preceding siblings ...)
  2019-01-14 22:03 ` [PATCH v2 7/9] platform/chrome: Add EC properties Nick Crews
@ 2019-01-14 22:03 ` Nick Crews
  2019-01-14 22:03 ` [PATCH v2 9/9] platform/chrome: Add binary telemetry attributes Nick Crews
  8 siblings, 0 replies; 17+ messages in thread
From: Nick Crews @ 2019-01-14 22:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: groeck, sjg, dlaurie, Nick Crews, Duncan Laurie,
	Enric Balletbo i Serra, Benson Leung

Create "peakshift" and "advanced_battery_charging" directories
within the "properties" directory, and create the relevant
attributes within these. These properties have to do with
configuring some of the advanced power management options that
prolong battery health and reduce energy use at peak hours
of the day.

Scheduling events uses a 24 hour clock, and only supports time
intervals of 15 minutes. For example, to set
advanced_battery_charging to start at 4:15pm and to last for
6 hours and 45 minutes, you would use the argument "16 15 6 45".

> cd /sys/bus/platform/devices/GOOG000C\:00
> cat properties/peakshift/peakshift_battery_threshold
> 015
[means 15 percent]
> cat properties/peakshift/peakshift_monday
16 00 20 30 00 00
[starts at 4:00 pm, ends at 8:30, charging resumes at midnight]
> echo "16 00 20 31 00 00" > properties/peakshift/peakshift_monday
-bash: echo: write error: Invalid argument
> dmesg | tail -n1
[40.34534] wilco_ec GOOG00C:00: minutes must be at the quarter hour
> echo "16 0 20 45 0 0" > properties/peakshift/peakshift_monday
> cat properties/peakshift/peakshift_monday
16 00 20 45 00 00

Signed-off-by: Nick Crews <ncrews@google.com>
---

Changes in v2:
- rm license boiler plate
- rm "wilco_ec_adv_power - " prefix from docstring
- Add documentation
- Made format strings in store() and read() functions static

 .../ABI/testing/sysfs-platform-wilcoec        |  75 +++
 drivers/platform/chrome/wilco_ec/Makefile     |   2 +-
 drivers/platform/chrome/wilco_ec/adv_power.c  | 544 ++++++++++++++++++
 drivers/platform/chrome/wilco_ec/adv_power.h  | 183 ++++++
 drivers/platform/chrome/wilco_ec/sysfs.c      | 100 ++++
 5 files changed, 903 insertions(+), 1 deletion(-)
 create mode 100644 drivers/platform/chrome/wilco_ec/adv_power.c
 create mode 100644 drivers/platform/chrome/wilco_ec/adv_power.h

diff --git a/Documentation/ABI/testing/sysfs-platform-wilcoec b/Documentation/ABI/testing/sysfs-platform-wilcoec
index b09e2473f948..2489b9e3fc99 100644
--- a/Documentation/ABI/testing/sysfs-platform-wilcoec
+++ b/Documentation/ABI/testing/sysfs-platform-wilcoec
@@ -93,3 +93,78 @@ Date:		January 2019
 KernelVersion:	4.19
 Description:
 		Enable early Keyboard BackLight at boot.
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/properties/peakshift/<day of week>
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		For each weekday a start and end time to run in Peak Shift mode
+		can be set. During these times the system will run from the
+		battery even if the AC is attached as long as the battery stays
+		above the threshold specified. After the end time specified the
+		system will run from AC if attached but will not charge the
+		battery. The system will again function normally using AC and
+		recharging the battery after the specified Charge Start time.
+
+		The input buffer must have the format "start_hr start_min end_hr
+		end_min charge_start_hr charge_start_min" The hour fields must
+		be in the range [0-23], and the minutes must be one of (0, 15,
+		30, 45). The string must be parseable by sscanf() using the
+		format string "%d %d %d %d %d %d". An example valid input is
+		"6 15   009 45 23 0", which corresponds to 6:15, 9:45, and 23:00
+
+		The output buffer will be filled with the format "start_hr
+		start_min end_hr end_min charge_start_hr charge_start_min" The
+		hour fields will be in the range [0-23], and the minutes will be
+		one of (0, 15, 30, 45). Each number will be zero padded to two
+		characters. An example output is "06 15 09 45 23 00", which
+		corresponds to 6:15, 9:45, and 23:00
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/properties/peakshift/enable
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Enable/disable the peakshift power management policy
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/properties/peakshift/battery_threshold
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Read/write the battery percentage threshold for which the
+		peakshift policy is used. The valid range is [15, 50].
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/properties/advanced_battery_charging/<day of week>
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Advanced Charging Mode allows the user to maximize the battery
+		health. In Advanced Charging Mode the system will use standard
+		charging algorithm and other techniques during non-work hours to
+		maximize battery health. During work hours, an express charge is
+		used. This express charge allows the battery to be charged
+		faster; therefore, the battery is at full charge sooner. For
+		each day the time in which the system will be most heavily used
+		is specified by the start time and the duration. Please read the
+		Common UEFI BIOS Behavioral Specification and BatMan 2 BIOS_EC
+		Specification for more details about this feature.
+
+		The input buffer must have the format "start_hr start_min
+		duration_hr duration_min" The hour fields must be in the range
+		[0-23], and the minutes must be one of (0, 15, 30, 45). The
+		string must be parseable by sscanf() using the format string
+		"%d %d %d %d %d %d". An example valid input is "0006 15  23 45",
+		which corresponds to a start time of 6:15 and a duration of
+		23:45.
+
+		The output buffer will be filled with the format "start_hr
+		start_min duration_hr duration_min" The hour fields will be in
+		the range [0-23], and the minutes will be one of (0, 15, 30,
+		45). Each number will be zero padded to two characters. An
+		example output is "06 15 23 45", which corresponds to a start
+		time of 6:15 and a duration of 23:45
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/properties/advanced_battery_charging/enable
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Enable/disable the Advanced Battery Charging policy
diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
index 9c9e67171652..5220a3ff63bd 100644
--- a/drivers/platform/chrome/wilco_ec/Makefile
+++ b/drivers/platform/chrome/wilco_ec/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 
 wilco_ec-objs				:= core.o mailbox.o sysfs.o legacy.o \
-					   event.o properties.o
+					   event.o properties.o adv_power.o
 obj-$(CONFIG_WILCO_EC)			+= wilco_ec.o
diff --git a/drivers/platform/chrome/wilco_ec/adv_power.c b/drivers/platform/chrome/wilco_ec/adv_power.c
new file mode 100644
index 000000000000..a26920235294
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/adv_power.c
@@ -0,0 +1,544 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * peakshift and adv_batt_charging config of Wilco EC
+ *
+ * Copyright 2018 Google LLC
+ *
+ * Peakshift:
+ * For each weekday a start and end time to run in Peak Shift mode can be set.
+ * During these times the system will run from the battery even if the AC is
+ * attached as long as the battery stays above the threshold specified.
+ * After the end time specified the system will run from AC if attached but
+ * will not charge the battery. The system will again function normally using AC
+ * and recharging the battery after the specified Charge Start time.
+ *
+ * Advanced Charging Mode:
+ * Advanced Charging Mode allows the user to maximize the battery health.
+ * In Advanced Charging Mode the system will use standard charging algorithm and
+ * other techniques during non-work hours to maximize battery health.
+ * During work hours, an express charge is used. This express charge allows the
+ * battery to be charged faster; therefore, the battery is at
+ * full charge sooner. For each day the time in which the system will be most
+ * heavily used is specified by the start time and the duration.
+ * Please read the Common UEFI BIOS Behavioral Specification and
+ * BatMan 2 BIOS_EC Specification for more details about this feature.
+ */
+
+#include <linux/kobject.h>
+#include <linux/device.h>
+#include <linux/platform_data/wilco-ec.h>
+
+#include "adv_power.h"
+#include "properties.h"
+#include "util.h"
+
+struct adv_batt_charging_data {
+	int duration_hours;
+	int duration_minutes;
+	int start_hours;
+	int start_minutes;
+};
+
+struct peakshift_data {
+	int start_hours;
+	int start_minutes;
+	int end_hours;
+	int end_minutes;
+	int charge_start_hours;
+	int charge_start_minutes;
+};
+
+/**
+ * struct time_bcd_format - spec for binary coded decimal time format
+ * @hour_position: how many bits left within the byte is the hour
+ * @minute_position: how many bits left within the byte is the minute
+ *
+ * Date and hour information is passed to/from the EC using packed bytes,
+ * where each byte represents an hour and a minute that some event occurs.
+ * The minute field always happens at quarter-hour intervals, so either
+ * 0, 15, 20, or 45. This allows this info to be packed within 2 bits.
+ * Along with the 5 bits of hour info [0-23], this gives us 7 used bits
+ * within each packed byte. The annoying thing is that the PEAKSHIFT and
+ * ADVANCED_BATTERY_CHARGING properties pack these 7 bits differently,
+ * hence this struct.
+ */
+struct time_bcd_format {
+	u8 hour_position;
+	u8 minute_position;
+};
+
+const struct time_bcd_format PEAKSHIFT_BCD_FORMAT = {
+			     // bit[0] is unused
+	.hour_position = 1,  // bits[1:7]
+	.minute_position = 6 // bits[6:8]
+};
+
+const struct time_bcd_format ADV_BATT_CHARGING_BCD_FORMAT = {
+	.minute_position = 0, // bits[0:2]
+	.hour_position = 2    // bits[2:7]
+			      // bit[7] is unused
+};
+
+/**
+ * struct peakshift_payload - The formatted peakshift time sent/received by EC.
+ * @start_time: packed byte of hour and minute info
+ * @end_time: packed byte of hour and minute info
+ * @charge_start_time: packed byte of hour and minute info
+ * @RESERVED: an unused padding byte
+ */
+struct peakshift_payload {
+	u8 start_time;
+	u8 end_time;
+	u8 charge_start_time;
+	u8 RESERVED;
+} __packed;
+
+struct adv_batt_charging_payload {
+	u16 RESERVED;
+	u8 duration_time;
+	u8 start_time;
+} __packed;
+
+/**
+ * extract_quarter_hour() - Convert from literal minutes to quarter hour.
+ * @minutes: Literal minutes value. Needs to be one of {0, 15, 30, 45}
+ *
+ * Return one of {0, 1, 2, 3} for each of {0, 15, 30, 45}, or -EINVAL on error.
+ */
+static int extract_quarter_hour(int minutes)
+{
+	if ((minutes < 0) || (minutes > 45) || minutes % 15)
+		return -EINVAL;
+	return minutes / 15;
+}
+
+static int check_adv_batt_charging_data(struct device *dev,
+					struct adv_batt_charging_data *data)
+{
+	if (data->start_hours < 0 || data->start_hours > 23) {
+		dev_err(dev, "start_hours must be in [0-23], got %d",
+			data->start_hours);
+		return -EINVAL;
+	}
+	if (data->duration_hours < 0 || data->duration_hours > 23) {
+		dev_err(dev, "duration_hours must be in [0-23], got %d",
+			data->duration_hours);
+		return -EINVAL;
+	}
+	if (data->start_minutes < 0 || data->start_minutes > 59) {
+		dev_err(dev, "start_minutes must be in [0-59], got %d",
+			data->start_minutes);
+		return -EINVAL;
+	}
+	if (data->duration_minutes < 0 || data->duration_minutes > 59) {
+		dev_err(dev, "duration_minutes must be in [0-59], got %d",
+			data->duration_minutes);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int check_peakshift_data(struct device *dev, struct peakshift_data *data)
+{
+	if (data->start_hours < 0 || data->start_hours > 23) {
+		dev_err(dev, "start_hours must be in [0-23], got %d",
+			data->start_hours);
+		return -EINVAL;
+	}
+	if (data->end_hours < 0 || data->end_hours > 23) {
+		dev_err(dev, "end_hours must be in [0-23], got %d",
+			data->end_hours);
+		return -EINVAL;
+	}
+	if (data->charge_start_hours < 0 || data->charge_start_hours > 23) {
+		dev_err(dev, "charge_start_hours must be in [0-23], got %d",
+			data->charge_start_hours);
+		return -EINVAL;
+	}
+	if (data->start_minutes < 0 || data->start_minutes > 59) {
+		dev_err(dev, "start_minutes must be in [0-59], got %d",
+			data->start_minutes);
+		return -EINVAL;
+	}
+	if (data->end_minutes < 0 || data->end_minutes > 59) {
+		dev_err(dev, "end_minutes must be in [0-59], got %d",
+			data->end_minutes);
+		return -EINVAL;
+	}
+	if (data->charge_start_minutes < 0 || data->charge_start_minutes > 59) {
+		dev_err(dev, "charge_start_minutes must be in [0-59], got %d",
+			data->charge_start_minutes);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+/**
+ * pack_field() - Pack hour and minute info into a byte.
+ *
+ * @fmt: The format for how to place the info within the byte
+ * @hours: In range [0-23]
+ * @quarter_hour: In range [0-3], representing :00, :15, :30, and :45
+ *
+ * Return the packed byte.
+ */
+static u8 pack_field(struct time_bcd_format fmt, int hours, int quarter_hour)
+{
+	int result = 0;
+
+	result |= hours << fmt.hour_position;
+	result |= quarter_hour << fmt.minute_position;
+	return (u8) result;
+}
+
+/**
+ * unpack_field() - Extract hour and minute info from a byte.
+ *
+ * @fmt: The format for how to place the info within the byte
+ * @field: Byte which contains the packed info
+ * @hours: The value to be filled, in [0, 24]
+ * @quarter_hour: to be filled in range [0-3], meaning :00, :15, :30, and :45
+ */
+static void unpack_field(struct time_bcd_format fmt, u8 field, int *hours,
+			 int *quarter_hour)
+{
+	*hours =	(field >> fmt.hour_position)   & 0x1f; // 00011111
+	*quarter_hour = (field >> fmt.minute_position) & 0x03; // 00000011
+}
+
+static void pack_adv_batt_charging(struct adv_batt_charging_data *data,
+		     struct adv_batt_charging_payload *payload)
+{
+	payload->start_time = pack_field(ADV_BATT_CHARGING_BCD_FORMAT,
+					 data->start_hours,
+					 data->start_minutes);
+	payload->duration_time = pack_field(ADV_BATT_CHARGING_BCD_FORMAT,
+				       data->duration_hours,
+				       data->duration_minutes);
+}
+
+static void unpack_adv_batt_charging(struct adv_batt_charging_data *data,
+		       struct adv_batt_charging_payload *payload)
+{
+	unpack_field(ADV_BATT_CHARGING_BCD_FORMAT, payload->start_time,
+		     &(data->start_hours),
+		     &(data->start_minutes));
+	unpack_field(ADV_BATT_CHARGING_BCD_FORMAT, payload->duration_time,
+		     &(data->duration_hours),
+		     &(data->duration_minutes));
+}
+
+static void pack_peakshift(struct peakshift_data *data,
+			   struct peakshift_payload *payload)
+{
+	payload->start_time = pack_field(PEAKSHIFT_BCD_FORMAT,
+					 data->start_hours,
+					 data->start_minutes);
+	payload->end_time = pack_field(PEAKSHIFT_BCD_FORMAT,
+				       data->end_hours,
+				       data->end_minutes);
+	payload->charge_start_time = pack_field(PEAKSHIFT_BCD_FORMAT,
+						data->charge_start_hours,
+						data->charge_start_minutes);
+}
+
+static void unpack_peakshift(struct peakshift_data *data,
+			     struct peakshift_payload *payload)
+{
+	unpack_field(PEAKSHIFT_BCD_FORMAT, payload->start_time,
+		     &(data->start_hours),
+		     &(data->start_minutes));
+	unpack_field(PEAKSHIFT_BCD_FORMAT, payload->end_time,
+		     &(data->end_hours),
+		     &(data->end_minutes));
+	unpack_field(PEAKSHIFT_BCD_FORMAT, payload->charge_start_time,
+		     &(data->charge_start_hours),
+		     &(data->charge_start_minutes));
+}
+
+ssize_t wilco_ec_peakshift_show(struct kobject *kobj,
+				struct kobj_attribute *attr, char *buf)
+{
+	struct property_attribute *prop_attr;
+	struct device *dev;
+	struct wilco_ec_device *ec;
+	struct peakshift_payload payload;
+	struct peakshift_data data;
+	static const char FORMAT[] = "%02d %02d %02d %02d %02d %02d\n";
+	const int OUT_LENGTH = 18; //six 2-char nums, 5 spaces, 1 newline
+	int ret;
+
+	if (OUT_LENGTH + 1 > PAGE_SIZE)
+		return -ENOBUFS; //no buffer space for message + null
+
+	prop_attr = container_of(attr, struct property_attribute, kobj_attr);
+	dev = device_from_kobject(kobj);
+	ec = dev_get_drvdata(dev);
+
+	/* get the raw payload of data from the EC */
+	ret = wilco_ec_get_property(ec, prop_attr->pid, sizeof(payload),
+				    (u8 *) &payload);
+	if (ret < 0) {
+		dev_err(dev, "error in wilco_ec_mailbox()");
+		return ret;
+	}
+
+	/* unpack raw bytes, and convert quarter-hour to literal minute */
+	unpack_peakshift(&data, &payload);
+	data.start_minutes *= 15;
+	data.end_minutes *= 15;
+	data.charge_start_minutes *= 15;
+
+	/* Check that the EC returns good data */
+	ret = check_peakshift_data(dev, &data);
+	if (ret < 0) {
+		dev_err(dev, "EC returned out of range minutes or hours");
+		return -EBADMSG;
+	}
+
+	/* Print the numbers to the string */
+	ret = scnprintf(buf, OUT_LENGTH+1, FORMAT,
+			data.start_hours,
+			data.start_minutes,
+			data.end_hours,
+			data.end_minutes,
+			data.charge_start_hours,
+			data.charge_start_minutes);
+	if (ret != OUT_LENGTH) {
+		dev_err(dev, "expected to write %d chars, wrote %d", OUT_LENGTH,
+			ret);
+		return -EIO;
+	}
+
+	return OUT_LENGTH;
+}
+
+ssize_t wilco_ec_peakshift_store(struct kobject *kobj,
+				 struct kobj_attribute *attr, const char *buf,
+				 size_t count)
+{
+	struct property_attribute *prop_attr;
+	struct device *dev;
+	struct wilco_ec_device *ec;
+	struct peakshift_data data;
+	struct peakshift_payload payload;
+	static const char FORMAT[] = "%d %d %d %d %d %d";
+	int ret;
+
+	prop_attr = container_of(attr, struct property_attribute, kobj_attr);
+	dev = device_from_kobject(kobj);
+	ec = dev_get_drvdata(dev);
+
+	/* Extract our 6 numbers from the input string */
+	ret = sscanf(buf, FORMAT,
+		     &data.start_hours,
+		     &data.start_minutes,
+		     &data.end_hours,
+		     &data.end_minutes,
+		     &data.charge_start_hours,
+		     &data.charge_start_minutes);
+	if (ret != 6) {
+		dev_err(dev, "unable to parse '%s' into 6 integers", buf);
+		return -EINVAL;
+	}
+
+	/* Ensure the integers we parsed are valid */
+	ret = check_peakshift_data(dev, &data);
+	if (ret < 0)
+		return ret;
+
+	/* Convert the literal minutes to which quarter hour they represent */
+	data.start_minutes = extract_quarter_hour(data.start_minutes);
+	if (data.start_minutes < 0)
+		goto bad_minutes;
+	data.end_minutes = extract_quarter_hour(data.end_minutes);
+	if (data.end_minutes < 0)
+		goto bad_minutes;
+	data.charge_start_minutes = extract_quarter_hour(
+						data.charge_start_minutes);
+	if (data.charge_start_minutes < 0)
+		goto bad_minutes;
+
+	/* Create the raw byte payload and send it off */
+	pack_peakshift(&data, &payload);
+	wilco_ec_set_property(ec, OP_SET, prop_attr->pid, sizeof(payload),
+			      (u8 *) &payload);
+
+	return count;
+
+bad_minutes:
+	dev_err(dev, "minutes must be at the quarter hour");
+	return -EINVAL;
+}
+
+ssize_t wilco_ec_peakshift_batt_thresh_show(struct kobject *kobj,
+					    struct kobj_attribute *attr,
+					    char *buf)
+{
+	struct device *dev;
+	struct wilco_ec_device *ec;
+	static const char FORMAT[] = "%02d\n";
+	size_t RESULT_LENGTH = 3; /* 2-char number and newline */
+	u8 percent;
+	int ret;
+
+	dev = device_from_kobject(kobj);
+	ec = dev_get_drvdata(dev);
+
+	ret = wilco_ec_get_property(ec, PID_PEAKSHIFT_BATTERY_THRESHOLD, 1,
+				    &percent);
+	if (ret < 0)
+		return ret;
+
+	if (percent < 15 || percent > 50) {
+		dev_err(ec->dev, "expected 15 < percentage < 50, got %d",
+			percent);
+		return -EBADMSG;
+	}
+
+	scnprintf(buf, RESULT_LENGTH+1, FORMAT, percent);
+
+	return RESULT_LENGTH;
+}
+
+ssize_t wilco_ec_peakshift_batt_thresh_store(struct kobject *kobj,
+					     struct kobj_attribute *attr,
+					     const char *buf, size_t count)
+{
+	struct device *dev;
+	struct wilco_ec_device *ec;
+	u8 DECIMAL_BASE = 10;
+	u8 percent;
+	int ret;
+
+	dev = device_from_kobject(kobj);
+	ec = dev_get_drvdata(dev);
+
+
+	ret = kstrtou8(buf, DECIMAL_BASE, &percent);
+	if (ret) {
+		dev_err(dev, "unable to parse '%s' to u8", buf);
+		return ret;
+	}
+
+	if (percent < 15 || percent > 50) {
+		dev_err(dev, "require 15 < batt_thresh_percent < 50, got %d",
+			percent);
+		return -EINVAL;
+	}
+
+	ret = wilco_ec_set_property(ec, OP_SET, PID_PEAKSHIFT_BATTERY_THRESHOLD,
+				    1, &percent);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+ssize_t wilco_ec_abc_show(struct kobject *kobj, struct kobj_attribute *attr,
+			  char *buf)
+{
+	struct property_attribute *prop_attr;
+	struct device *dev;
+	struct wilco_ec_device *ec;
+	struct adv_batt_charging_payload payload;
+	struct adv_batt_charging_data data;
+	static const char FORMAT[] = "%02d %02d %02d %02d\n";
+	const int OUT_LENGTH = 12; //four 2-char nums, 3 spaces, 1 newline
+	int ret;
+
+	prop_attr = container_of(attr, struct property_attribute, kobj_attr);
+	dev = device_from_kobject(kobj);
+	ec = dev_get_drvdata(dev);
+
+
+	if (OUT_LENGTH + 1 > PAGE_SIZE)
+		return -ENOBUFS; //no buffer space for message + null
+
+	/* get the raw payload of data from the EC */
+	ret = wilco_ec_get_property(ec, prop_attr->pid, sizeof(payload),
+				    (u8 *) &payload);
+	if (ret < 0) {
+		dev_err(dev, "error in wilco_ec_mailbox()");
+		return ret;
+	}
+
+	/* unpack raw bytes, and convert quarter-hour to literal minute */
+	unpack_adv_batt_charging(&data, &payload);
+	data.start_minutes *= 15;
+	data.duration_minutes *= 15;
+
+	// /* Is this needed? can we assume the EC returns good data? */
+	// EC is returning 00 00 27 30. Was this modified, or is EC weird
+	// out of the box?
+	ret = check_adv_batt_charging_data(dev, &data);
+	if (ret < 0) {
+		dev_err(dev, "EC returned out of range minutes or hours");
+		return -EBADMSG;
+	}
+
+	/* Print the numbers to the string */
+	ret = scnprintf(buf, OUT_LENGTH+1, FORMAT,
+			data.start_hours,
+			data.start_minutes,
+			data.duration_hours,
+			data.duration_minutes);
+	if (ret != OUT_LENGTH) {
+		dev_err(dev, "expected to write %d chars, wrote %d", OUT_LENGTH,
+			ret);
+		return -EIO;
+	}
+
+	return OUT_LENGTH;
+}
+
+ssize_t wilco_ec_abc_store(struct kobject *kobj, struct kobj_attribute *attr,
+			   const char *buf, size_t count)
+{
+	struct property_attribute *prop_attr;
+	struct device *dev;
+	struct wilco_ec_device *ec;
+	struct adv_batt_charging_data data;
+	struct adv_batt_charging_payload payload;
+	static const char FORMAT[] = "%d %d %d %d";
+	int ret;
+
+	prop_attr = container_of(attr, struct property_attribute, kobj_attr);
+	dev = device_from_kobject(kobj);
+	ec = dev_get_drvdata(dev);
+
+	/* Extract our 4 numbers from the input string */
+	ret = sscanf(buf, FORMAT,
+		     &data.start_hours,
+		     &data.start_minutes,
+		     &data.duration_hours,
+		     &data.duration_minutes);
+	if (ret != 4) {
+		dev_err(dev, "unable to parse '%s' into 4 integers", buf);
+		return -EINVAL;
+	}
+
+	/* Ensure the integers we parsed are valid */
+	ret = check_adv_batt_charging_data(dev, &data);
+	if (ret < 0)
+		return ret;
+
+	/* Convert the literal minutes to which quarter hour they represent */
+	data.start_minutes = extract_quarter_hour(data.start_minutes);
+	if (data.start_minutes < 0)
+		goto bad_minutes;
+	data.duration_minutes = extract_quarter_hour(data.duration_minutes);
+	if (data.duration_minutes < 0)
+		goto bad_minutes;
+
+	/* Create the raw byte payload and send it off */
+	pack_adv_batt_charging(&data, &payload);
+	wilco_ec_set_property(ec, OP_SET, prop_attr->pid, sizeof(payload),
+			      (u8 *) &payload);
+
+	return count;
+
+bad_minutes:
+	dev_err(dev, "minutes must be at the quarter hour");
+	return -EINVAL;
+}
diff --git a/drivers/platform/chrome/wilco_ec/adv_power.h b/drivers/platform/chrome/wilco_ec/adv_power.h
new file mode 100644
index 000000000000..6fd64c6dac8f
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/adv_power.h
@@ -0,0 +1,183 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * peakshift and adv_batt_charging config of Wilco EC
+ *
+ * Copyright 2018 Google LLC
+ *
+ * Peakshift:
+ * For each weekday a start and end time to run in Peak Shift mode can be set.
+ * During these times the system will run from the battery even if the AC is
+ * attached as long as the battery stays above the threshold specified.
+ * After the end time specified the system will run from AC if attached but
+ * will not charge the battery. The system will again function normally using AC
+ * and recharging the battery after the specified Charge Start time.
+ *
+ * Advanced Charging Mode:
+ * Advanced Charging Mode allows the user to maximize the battery health.
+ * In Advanced Charging Mode the system will use standard charging algorithm and
+ * other techniques during non-work hours to maximize battery health.
+ * During work hours, an express charge is used. This express charge allows the
+ * battery to be charged faster; therefore, the battery is at
+ * full charge sooner. For each day the time in which the system will be most
+ * heavily used is specified by the start time and the duration.
+ * Please read the Common UEFI BIOS Behavioral Specification and
+ * BatMan 2 BIOS_EC Specification for more details about this feature.
+ */
+
+#ifndef WILCO_EC_ADV_POWER_H
+#define WILCO_EC_ADV_POWER_H
+
+#include <linux/kobject.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/sysfs.h>
+
+#include "util.h"
+#include "properties.h"
+
+#define PID_PEAKSHIFT				0x0412
+#define PID_PEAKSHIFT_BATTERY_THRESHOLD		0x04EB
+#define PID_PEAKSHIFT_SUNDAY_HOURS		0x04F5
+#define PID_PEAKSHIFT_MONDAY_HOURS		0x04F6
+#define PID_PEAKSHIFT_TUESDAY_HOURS		0x04F7
+#define PID_PEAKSHIFT_WEDNESDAY_HOURS		0x04F8
+#define PID_PEAKSHIFT_THURSDAY_HOURS		0x04F9
+#define PID_PEAKSHIFT_FRIDAY_HOURS		0x04Fa
+#define PID_PEAKSHIFT_SATURDAY_HOURS		0x04Fb
+
+#define PID_ABC_MODE				0x04ed
+#define PID_ABC_SUNDAY_HOURS			0x04F5
+#define PID_ABC_MONDAY_HOURS			0x04F6
+#define PID_ABC_TUESDAY_HOURS			0x04F7
+#define PID_ABC_WEDNESDAY_HOURS			0x04F8
+#define PID_ABC_THURSDAY_HOURS			0x04F9
+#define PID_ABC_FRIDAY_HOURS			0x04FA
+#define PID_ABC_SATURDAY_HOURS			0x04FB
+
+/**
+ * wilco_ec_peakshift_show() - Retrieves times stored for the peakshift policy.
+ * @kobj: kobject representing the directory this attribute is in
+ * @attr: Attribute stored within the proper property_attribute
+ * @buf: Output buffer to fill with the result
+ *
+ * The output buffer will be filled with the format
+ * "start_hr start_min end_hr end_min charge_start_hr charge_start_min"
+ * The hour fields will be in the range [0-23], and the minutes will be
+ * one of (0, 15, 30, 45). Each number will be zero padded to two characters.
+ *
+ * An example output is "06 15 09 45 23 00",
+ * which corresponds to 6:15, 9:45, and 23:00
+ *
+ * Return the length of the output buffer, or negative error code on failure.
+ */
+ssize_t wilco_ec_peakshift_show(struct kobject *kobj,
+				struct kobj_attribute *attr, char *buf);
+
+/**
+ * wilco_ec_peakshift_store() - Saves times for the peakshift policy.
+ * @kobj: kobject representing the directory this attribute is in
+ * @attr: Attribute stored within the proper property_attribute
+ * @buf: Raw input buffer
+ * @count: Number of bytes in input buffer
+ *
+ * The input buffer must have the format
+ * "start_hr start_min end_hr end_min charge_start_hr charge_start_min"
+ * The hour fields must be in the range [0-23], and the minutes must be
+ * one of (0, 15, 30, 45). The string must be parseable by sscanf() using the
+ * format string "%d %d %d %d %d %d".
+ *
+ * An example valid input is "6 15     009 45 23 0",
+ * which corresponds to 6:15, 9:45, and 23:00
+ *
+ * Return number of bytes consumed from input, negative error code on failure.
+ */
+ssize_t wilco_ec_peakshift_store(struct kobject *kobj,
+				 struct kobj_attribute *attr, const char *buf,
+				 size_t count);
+
+/**
+ * peakshift_battery_show() - Retrieve batt percentage at which peakshift stops
+ * @kobj: kobject representing the directory this attribute is in
+ * @attr: Attribute stored within the proper property_attribute
+ * @buf: Output buffer to fill with the result
+ *
+ * Result will be a 2 character integer representing the
+ * battery percentage at which peakshift stops. Will be in range [15, 50].
+ *
+ * Return the length of the output buffer, or negative error code on failure.
+ */
+ssize_t wilco_ec_peakshift_batt_thresh_show(struct kobject *kobj,
+					    struct kobj_attribute *attr,
+					    char *buf);
+
+/**
+ * peakshift_battery_store() - Save batt percentage at which peakshift stops
+ * @kobj: kobject representing the directory this attribute is in
+ * @attr: Attribute stored within the proper property_attribute
+ * @buf: Input buffer, should be parseable to range [15,50] by kstrtou8()
+ * @count: Number of bytes in input buffer
+ *
+ * Return number of bytes consumed from input, negative error code on failure.
+ */
+ssize_t wilco_ec_peakshift_batt_thresh_store(struct kobject *kobj,
+					     struct kobj_attribute *attr,
+					     const char *buf, size_t count);
+
+/**
+ * wilco_ec_abc_show() - Retrieve times for Advanced Battery Charging
+ * @kobj: kobject representing the directory this attribute is in
+ * @attr: Attribute stored within the proper property_attribute
+ * @buf: Output buffer to fill with the result
+ *
+ * The output buffer will be filled with the format
+ * "start_hr start_min duration_hr duration_min"
+ * The hour fields will be in the range [0-23], and the minutes will be
+ * one of (0, 15, 30, 45). Each number will be zero padded to two characters.
+ *
+ * An example output is "06 15 23 45",
+ * which corresponds to a start time of 6:15 and a duration of 23:45
+ *
+ * Return the length of the output buffer, or negative error code on failure.
+ */
+ssize_t wilco_ec_abc_show(struct kobject *kobj, struct kobj_attribute *attr,
+			  char *buf);
+
+/**
+ * wilco_ec_abc_store() - Save times for Advanced Battery Charging
+ * @kobj: kobject representing the directory this attribute is in
+ * @attr: Attribute stored within the proper property_attribute
+ * @buf: The raw input buffer
+ * @count: Number of bytes in input buffer
+ *
+ * The input buffer must have the format
+ * "start_hr start_min duration_hr duration_min"
+ * The hour fields must be in the range [0-23], and the minutes must be
+ * one of (0, 15, 30, 45). The string must be parseable by sscanf() using the
+ * format string "%d %d %d %d %d %d".
+ *
+ * An example valid input is "0006 15     23 45",
+ * which corresponds to a start time of 6:15 and a duration of 23:45
+ *
+ * Return number of bytes consumed, or negative error code on failure.
+ */
+ssize_t wilco_ec_abc_store(struct kobject *kobj, struct kobj_attribute *attr,
+			   const char *buf, size_t count);
+
+#define PEAKSHIFT_KOBJ_ATTR(_name)					\
+__ATTR(_name, 0644, wilco_ec_peakshift_show, wilco_ec_peakshift_store)
+
+#define PEAKSHIFT_ATTRIBUTE(_var, _name, _pid)				\
+struct property_attribute _var = {					\
+	.kobj_attr = PEAKSHIFT_KOBJ_ATTR(_name),			\
+	.pid = _pid,							\
+}
+
+#define ABC_KOBJ_ATTR(_name)						\
+__ATTR(_name, 0644, wilco_ec_abc_show, wilco_ec_abc_store)
+
+#define ABC_ATTRIBUTE(_var, _name, _pid)				\
+struct property_attribute _var = {					\
+	.kobj_attr = ABC_KOBJ_ATTR(_name),				\
+	.pid = _pid,							\
+}
+
+#endif /* WILCO_EC_ADV_POWER_H */
diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c b/drivers/platform/chrome/wilco_ec/sysfs.c
index b81f251f78fc..c5657bf7672a 100644
--- a/drivers/platform/chrome/wilco_ec/sysfs.c
+++ b/drivers/platform/chrome/wilco_ec/sysfs.c
@@ -16,6 +16,7 @@
 
 #include "legacy.h"
 #include "properties.h"
+#include "adv_power.h"
 
 #define WILCO_EC_ATTR_RO(_name)						\
 __ATTR(_name, 0444, wilco_ec_##_name##_show, NULL)
@@ -74,6 +75,67 @@ struct attribute *wilco_ec_property_attrs[] = {
 ATTRIBUTE_GROUPS(wilco_ec_property);
 struct kobject *prop_dir_kobj;
 
+/* Make peakshift attrs, which live inside GOOG000C:00/properties/peakshift */
+struct kobj_attribute kobj_attr_peakshift_battery_threshold =
+	__ATTR(battery_threshold, 0644, wilco_ec_peakshift_batt_thresh_show,
+	       wilco_ec_peakshift_batt_thresh_store);
+BOOLEAN_PROPERTY_RW_ATTRIBUTE(OP_SET, prop_attr_peakshift, enable,
+			      PID_PEAKSHIFT);
+PEAKSHIFT_ATTRIBUTE(prop_attr_peakshift_sunday, sunday,
+		    PID_PEAKSHIFT_SUNDAY_HOURS);
+PEAKSHIFT_ATTRIBUTE(prop_attr_peakshift_monday, monday,
+		    PID_PEAKSHIFT_MONDAY_HOURS);
+PEAKSHIFT_ATTRIBUTE(prop_attr_peakshift_tuesday, tuesday,
+		    PID_PEAKSHIFT_TUESDAY_HOURS);
+PEAKSHIFT_ATTRIBUTE(prop_attr_peakshift_wednesday, wednesday,
+		    PID_PEAKSHIFT_WEDNESDAY_HOURS);
+PEAKSHIFT_ATTRIBUTE(prop_attr_peakshift_thursday, thursday,
+		    PID_PEAKSHIFT_THURSDAY_HOURS);
+PEAKSHIFT_ATTRIBUTE(prop_attr_peakshift_friday, friday,
+		    PID_PEAKSHIFT_FRIDAY_HOURS);
+PEAKSHIFT_ATTRIBUTE(prop_attr_peakshift_saturday, saturday,
+		    PID_PEAKSHIFT_SATURDAY_HOURS);
+struct attribute *wilco_ec_peakshift_attrs[] = {
+	&kobj_attr_peakshift_battery_threshold.attr,
+	&prop_attr_peakshift.kobj_attr.attr,
+	&prop_attr_peakshift_sunday.kobj_attr.attr,
+	&prop_attr_peakshift_monday.kobj_attr.attr,
+	&prop_attr_peakshift_tuesday.kobj_attr.attr,
+	&prop_attr_peakshift_wednesday.kobj_attr.attr,
+	&prop_attr_peakshift_thursday.kobj_attr.attr,
+	&prop_attr_peakshift_friday.kobj_attr.attr,
+	&prop_attr_peakshift_saturday.kobj_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(wilco_ec_peakshift);
+struct kobject *peakshift_dir_kobj;
+
+/**
+ * Make peakshift attrs, which live inside
+ * GOOG000C:00/properties/advanced_battery_charging
+ */
+BOOLEAN_PROPERTY_RW_ATTRIBUTE(OP_SET, prop_attr_abc, enable, PID_ABC_MODE);
+ABC_ATTRIBUTE(prop_attr_abc_sunday, sunday, PID_ABC_SUNDAY_HOURS);
+ABC_ATTRIBUTE(prop_attr_abc_monday, monday, PID_ABC_MONDAY_HOURS);
+ABC_ATTRIBUTE(prop_attr_abc_tuesday, tuesday, PID_ABC_TUESDAY_HOURS);
+ABC_ATTRIBUTE(prop_attr_abc_wednesday, wednesday, PID_ABC_WEDNESDAY_HOURS);
+ABC_ATTRIBUTE(prop_attr_abc_thursday, thursday, PID_ABC_THURSDAY_HOURS);
+ABC_ATTRIBUTE(prop_attr_abc_friday, friday, PID_ABC_FRIDAY_HOURS);
+ABC_ATTRIBUTE(prop_attr_abc_saturday, saturday, PID_ABC_SATURDAY_HOURS);
+struct attribute *wilco_ec_adv_batt_charging_attrs[] = {
+	&prop_attr_abc.kobj_attr.attr,
+	&prop_attr_abc_sunday.kobj_attr.attr,
+	&prop_attr_abc_monday.kobj_attr.attr,
+	&prop_attr_abc_tuesday.kobj_attr.attr,
+	&prop_attr_abc_wednesday.kobj_attr.attr,
+	&prop_attr_abc_thursday.kobj_attr.attr,
+	&prop_attr_abc_friday.kobj_attr.attr,
+	&prop_attr_abc_saturday.kobj_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(wilco_ec_adv_batt_charging);
+struct kobject *adv_batt_charging_dir_kobj;
+
 /**
  * wilco_ec_sysfs_init() - Initialize the sysfs directories and attributes
  * @dev: The device representing the EC
@@ -103,9 +165,42 @@ int wilco_ec_sysfs_init(struct wilco_ec_device *ec)
 	if (ret)
 		goto rm_properties_dir;
 
+	/* add directory for adv batt charging into the properties directory */
+	adv_batt_charging_dir_kobj = kobject_create_and_add(
+					"advanced_battery_charging",
+					prop_dir_kobj);
+	if (!adv_batt_charging_dir_kobj)
+		goto rm_properties_attrs;
+
+	/* add the adv batt charging attributes into the abc directory */
+	ret = sysfs_create_groups(adv_batt_charging_dir_kobj,
+				  wilco_ec_adv_batt_charging_groups);
+	if (ret)
+		goto rm_abc_dir;
+
+	/* add the directory for peakshift into the properties directory */
+	peakshift_dir_kobj = kobject_create_and_add("peakshift", prop_dir_kobj);
+	if (!peakshift_dir_kobj)
+		goto rm_abc_attrs;
+
+	/* add the peakshift attributes into the peakshift directory */
+	ret = sysfs_create_groups(peakshift_dir_kobj,
+				  wilco_ec_peakshift_groups);
+	if (ret)
+		goto rm_peakshift_dir;
+
 	return 0;
 
 /* Go upwards through the directory structure, cleaning up */
+rm_peakshift_dir:
+	kobject_put(peakshift_dir_kobj);
+rm_abc_attrs:
+	sysfs_remove_groups(adv_batt_charging_dir_kobj,
+			    wilco_ec_adv_batt_charging_groups);
+rm_abc_dir:
+	kobject_put(adv_batt_charging_dir_kobj);
+rm_properties_attrs:
+	sysfs_remove_groups(prop_dir_kobj, wilco_ec_property_groups);
 rm_properties_dir:
 	kobject_put(prop_dir_kobj);
 rm_toplevel_attrs:
@@ -119,6 +214,11 @@ void wilco_ec_sysfs_remove(struct wilco_ec_device *ec)
 	struct device *dev = ec->dev;
 
 	/* go upwards through the directory structure */
+	sysfs_remove_groups(peakshift_dir_kobj, wilco_ec_peakshift_groups);
+	kobject_put(peakshift_dir_kobj);
+	sysfs_remove_groups(adv_batt_charging_dir_kobj,
+			    wilco_ec_adv_batt_charging_groups);
+	kobject_put(adv_batt_charging_dir_kobj);
 	sysfs_remove_groups(prop_dir_kobj, wilco_ec_property_groups);
 	kobject_put(prop_dir_kobj);
 	sysfs_remove_groups(&dev->kobj, wilco_ec_toplevel_groups);
-- 
2.20.1.97.g81188d93c3-goog


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

* [PATCH v2 9/9] platform/chrome: Add binary telemetry attributes
  2019-01-14 22:03 [PATCH v2 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
                   ` (7 preceding siblings ...)
  2019-01-14 22:03 ` [PATCH v2 8/9] platform/chrome: Add peakshift and adv_batt_charging Nick Crews
@ 2019-01-14 22:03 ` Nick Crews
  8 siblings, 0 replies; 17+ messages in thread
From: Nick Crews @ 2019-01-14 22:03 UTC (permalink / raw)
  To: linux-kernel
  Cc: groeck, sjg, dlaurie, Nick Crews, Duncan Laurie,
	Enric Balletbo i Serra, Benson Leung

The Wilco Embedded Controller is able to send telemetry data
which is useful for enterprise applications. A daemon running on
the OS sends a command (possibly with args) to the EC via
this sysfs interface, and the EC responds over the sysfs interface
with the response. Both the request and the response are in an opaque
binary format so that information which is proprietary to the
enterprise service provider is secure.

At this point, the Wilco EC does not implement this telemetry
functionality, so any request using the WILCO_EC_MSG_TELEMETRY
command returns an error. This is just an initial change for
comments, until the EC code is implemented.

Signed-off-by: Nick Crews <ncrews@google.com>
---

Changes in v2:
- rm "wilco_ec_telemetry - " prefix from docstring
- rm license boiler plate
- Fix commit msg tag

 .../ABI/testing/sysfs-platform-wilcoec        |  6 ++
 drivers/platform/chrome/wilco_ec/Makefile     |  3 +-
 drivers/platform/chrome/wilco_ec/sysfs.c      | 15 ++++-
 drivers/platform/chrome/wilco_ec/telemetry.c  | 66 +++++++++++++++++++
 drivers/platform/chrome/wilco_ec/telemetry.h  | 41 ++++++++++++
 5 files changed, 129 insertions(+), 2 deletions(-)
 create mode 100644 drivers/platform/chrome/wilco_ec/telemetry.c
 create mode 100644 drivers/platform/chrome/wilco_ec/telemetry.h

diff --git a/Documentation/ABI/testing/sysfs-platform-wilcoec b/Documentation/ABI/testing/sysfs-platform-wilcoec
index 2489b9e3fc99..de1fcb1a6b7c 100644
--- a/Documentation/ABI/testing/sysfs-platform-wilcoec
+++ b/Documentation/ABI/testing/sysfs-platform-wilcoec
@@ -168,3 +168,9 @@ Date:		January 2019
 KernelVersion:	4.19
 Description:
 		Enable/disable the Advanced Battery Charging policy
+
+What:		/sys/bus/platform/devices/GOOG000C\:00/telemetry
+Date:		January 2019
+KernelVersion:	4.19
+Description:
+		Send and receive opaque binary telemetry data to/from the EC.
diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
index 5220a3ff63bd..7a10501123c5 100644
--- a/drivers/platform/chrome/wilco_ec/Makefile
+++ b/drivers/platform/chrome/wilco_ec/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 
 wilco_ec-objs				:= core.o mailbox.o sysfs.o legacy.o \
-					   event.o properties.o adv_power.o
+					   event.o properties.o adv_power.o \
+					   telemetry.o
 obj-$(CONFIG_WILCO_EC)			+= wilco_ec.o
diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c b/drivers/platform/chrome/wilco_ec/sysfs.c
index c5657bf7672a..f8c4b0870cf2 100644
--- a/drivers/platform/chrome/wilco_ec/sysfs.c
+++ b/drivers/platform/chrome/wilco_ec/sysfs.c
@@ -17,6 +17,7 @@
 #include "legacy.h"
 #include "properties.h"
 #include "adv_power.h"
+#include "telemetry.h"
 
 #define WILCO_EC_ATTR_RO(_name)						\
 __ATTR(_name, 0444, wilco_ec_##_name##_show, NULL)
@@ -41,7 +42,19 @@ static struct attribute *wilco_ec_toplevel_attrs[] = {
 #endif
 	NULL
 };
-ATTRIBUTE_GROUPS(wilco_ec_toplevel);
+static struct bin_attribute telem_attr = TELEMETRY_BIN_ATTR(telemetry);
+static struct bin_attribute *telem_attrs[] = {
+	&telem_attr,
+	NULL
+};
+static const struct attribute_group wilco_ec_toplevel_group = {
+	.attrs = wilco_ec_toplevel_attrs,
+	.bin_attrs = telem_attrs,
+};
+static const struct attribute_group *wilco_ec_toplevel_groups[] = {
+	&wilco_ec_toplevel_group,
+	NULL,
+};
 
 /* Make property attributes, which will live inside GOOG000C:00/properties/  */
 BOOLEAN_PROPERTY_RW_ATTRIBUTE(OP_SET, bool_prop_attr_global_mic_mute_led,
diff --git a/drivers/platform/chrome/wilco_ec/telemetry.c b/drivers/platform/chrome/wilco_ec/telemetry.c
new file mode 100644
index 000000000000..cf3ae92bdab7
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/telemetry.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Telemetry sysfs attributes for Wilco EC
+ *
+ * Copyright 2018 Google LLC
+ *
+ * The Wilco Embedded Controller is able to send telemetry data
+ * which is useful for enterprise applications. A daemon running on
+ * the OS sends a command (possibly with args) to the EC via
+ * this sysfs interface, and the EC responds over the sysfs interface
+ * with the response. Both the request and the response are in an opaque
+ * binary format so that information which is proprietary to the
+ * enterprise service provider is secure.
+ */
+
+#include <linux/fs.h>
+#include <linux/kobject.h>
+#include <linux/device.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
+#include "util.h"
+
+/* Data buffer for holding EC's response for telemtry data */
+static u8 telemetry_data[EC_MAILBOX_DATA_SIZE_EXTENDED];
+
+ssize_t wilco_ec_telem_write(struct file *filp, struct kobject *kobj,
+			     struct bin_attribute *attr, char *buf, loff_t loff,
+			     size_t count)
+{
+	struct wilco_ec_message msg;
+	int ret;
+	struct device *dev = device_from_kobject(kobj);
+	struct wilco_ec_device *ec = dev_get_drvdata(dev);
+
+	if (count < 1 || count > EC_MAILBOX_DATA_SIZE_EXTENDED)
+		return -EINVAL;
+
+	/* Clear response data buffer */
+	memset(telemetry_data, 0, EC_MAILBOX_DATA_SIZE_EXTENDED);
+
+	msg.type = WILCO_EC_MSG_TELEMETRY;
+	msg.flags = WILCO_EC_FLAG_RAW | WILCO_EC_FLAG_EXTENDED_DATA;
+	msg.command = buf[0];
+	msg.request_data = buf + 1;
+	msg.request_size = EC_MAILBOX_DATA_SIZE;
+	msg.response_data = &telemetry_data;
+	msg.response_size = EC_MAILBOX_DATA_SIZE_EXTENDED;
+
+	/* Send the requested command + data as raw transaction */
+	ret = wilco_ec_mailbox(ec, &msg);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+ssize_t wilco_ec_telem_read(struct file *filp, struct kobject *kobj,
+			    struct bin_attribute *attr, char *buf, loff_t off,
+			    size_t count)
+{
+	memcpy(buf, telemetry_data, min_t(unsigned long, count,
+	       EC_MAILBOX_DATA_SIZE_EXTENDED));
+	return count;
+}
diff --git a/drivers/platform/chrome/wilco_ec/telemetry.h b/drivers/platform/chrome/wilco_ec/telemetry.h
new file mode 100644
index 000000000000..d68cc067b97a
--- /dev/null
+++ b/drivers/platform/chrome/wilco_ec/telemetry.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Telemetry sysfs attributes for Wilco EC
+ *
+ * Copyright 2018 Google LLC
+ *
+ * The Wilco Embedded Controller is able to send telemetry data
+ * which is useful for enterprise applications. A daemon running on
+ * the OS sends a command (possibly with args) to the EC via
+ * this sysfs interface, and the EC responds over the sysfs interface
+ * with the response. Both the request and the response are in an opaque
+ * binary format so that information which is proprietary to the
+ * enterprise service provider is secure.
+ */
+
+#ifndef WILCO_EC_TELEMETRY_H
+#define WILCO_EC_TELEMETRY_H
+
+#include <linux/fs.h>
+#include <linux/kobject.h>
+#include <linux/platform_data/wilco-ec.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
+ssize_t wilco_ec_telem_write(struct file *filp, struct kobject *kobj,
+			     struct bin_attribute *attr, char *buf, loff_t loff,
+			     size_t count);
+
+ssize_t wilco_ec_telem_read(struct file *filp, struct kobject *kobj,
+			    struct bin_attribute *attr, char *buf, loff_t off,
+			    size_t count);
+
+#define TELEMETRY_BIN_ATTR(_name) {					\
+	.attr = {.name = __stringify(_name),				\
+		 .mode = VERIFY_OCTAL_PERMISSIONS(0644) },		\
+	.size = EC_MAILBOX_DATA_SIZE_EXTENDED,				\
+	.read = wilco_ec_telem_read,					\
+	.write = wilco_ec_telem_write,					\
+}
+
+#endif /* WILCO_EC_TELEMETRY_H */
-- 
2.20.1.97.g81188d93c3-goog


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

* Re: [PATCH v2 5/9] platform/chrome: rtc: Add RTC driver for Wilco EC
  2019-01-14 22:03 ` [PATCH v2 5/9] platform/chrome: rtc: Add RTC driver for Wilco EC Nick Crews
@ 2019-01-14 22:25   ` Alexandre Belloni
  2019-01-15  0:30     ` Nick Crews
  2019-01-18 21:24     ` Nick Crews
  0 siblings, 2 replies; 17+ messages in thread
From: Alexandre Belloni @ 2019-01-14 22:25 UTC (permalink / raw)
  To: Nick Crews
  Cc: linux-kernel, groeck, sjg, dlaurie, Duncan Laurie, linux-rtc,
	Enric Balletbo i Serra, Alessandro Zummo, Benson Leung

Hello,

On 14/01/2019 15:03:52-0700, Nick Crews wrote:
> diff --git a/drivers/rtc/rtc-wilco-ec.c b/drivers/rtc/rtc-wilco-ec.c
> new file mode 100644
> index 000000000000..d8ed791da82d
> --- /dev/null
> +++ b/drivers/rtc/rtc-wilco-ec.c
> @@ -0,0 +1,178 @@
> +// 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 DRV_NAME "rtc-wilco-ec"

Please get rid of that define.

> +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),
> +	};
> +	struct rtc_time calc_tm;
> +	unsigned long time;
> +	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;
> +	/*
> +	 * The RTC stores the month value as 1-12 but the kernel expects 0-11,
> +	 * so ensure invalid/zero month value from RTC is not converted to -1.

I'm pretty sure this is not necessary and will certainly make the core
think the date is valid when it has obviously been corrupted.

> +	 */
> +	tm->tm_mon	= rtc.month ? rtc.month - 1 : 0;
> +	tm->tm_year	= rtc.year + (rtc.century * 100) - 1900;
> +	tm->tm_yday	= rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year);
> +
> +	/* Compute day of week */
> +	rtc_tm_to_time(tm, &time);
> +	rtc_time_to_tm(time, &calc_tm);
> +	tm->tm_wday = calc_tm.tm_wday;

Do you really need an accurate wday? There are no userspace applications
that I know of that care and converting back and forth is quite
expensive.

> +
> +	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 */
> +	int wday = tm->tm_wday == 6 ? 0 : tm->tm_wday + 1;

I'm not sure how useful it is to set an accurate wday when the RTC is
not able to give it back.

> +	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 = devm_rtc_device_register(&pdev->dev,
> +							  DRV_NAME,
> +							  &wilco_ec_rtc_ops,
> +							  THIS_MODULE);

Please use devm_rtc_allocate_device() and set rtc->range_min and
rtc->range_max before registering with rtc_register_device().

> +	if (IS_ERR(rtc))
> +		return PTR_ERR(rtc);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver wilco_ec_rtc_driver = {
> +	.driver = {
> +		.name = DRV_NAME,
> +	},
> +	.probe = wilco_ec_rtc_probe,
> +};
> +
> +module_platform_driver(wilco_ec_rtc_driver);
> +
> +MODULE_ALIAS("platform:" DRV_NAME);
> +MODULE_AUTHOR("Nick Crews <ncrews@google.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Wilco EC RTC driver");
> -- 
> 2.20.1.97.g81188d93c3-goog
> 

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

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

* Re: [PATCH v2 5/9] platform/chrome: rtc: Add RTC driver for Wilco EC
  2019-01-14 22:25   ` Alexandre Belloni
@ 2019-01-15  0:30     ` Nick Crews
  2019-01-18 21:24     ` Nick Crews
  1 sibling, 0 replies; 17+ messages in thread
From: Nick Crews @ 2019-01-15  0:30 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: linux-kernel, groeck, sjg, dlaurie, Duncan Laurie, linux-rtc,
	Enric Balletbo i Serra, Alessandro Zummo, Benson Leung

Thanks for the comments Alexandre, I've responded to your comments
inline. I'll send out a new version of the patch in a bit

On Mon, Jan 14, 2019 at 3:26 PM -700 Alexandre Belloni wrote:
>
> Hello,
>
> On 14/01/2019 15:03:52-0700, Nick Crews wrote:
> > diff --git a/drivers/rtc/rtc-wilco-ec.c b/drivers/rtc/rtc-wilco-ec.c
> > new file mode 100644
> > index 000000000000..d8ed791da82d
> > --- /dev/null
> > +++ b/drivers/rtc/rtc-wilco-ec.c
> > @@ -0,0 +1,178 @@
> > +// 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 DRV_NAME "rtc-wilco-ec"
>
> Please get rid of that define.

Done in the new patch.

>
> > +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),
> > +     };
> > +     struct rtc_time calc_tm;
> > +     unsigned long time;
> > +     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;
> > +     /*
> > +      * The RTC stores the month value as 1-12 but the kernel expects 0-11,
> > +      * so ensure invalid/zero month value from RTC is not converted to -1.
>
> I'm pretty sure this is not necessary and will certainly make the core
> think the date is valid when it has obviously been corrupted.

OK, I'll get rid of the safety check and just do rtc.month -1.

>
> > +      */
> > +     tm->tm_mon      = rtc.month ? rtc.month - 1 : 0;
> > +     tm->tm_year     = rtc.year + (rtc.century * 100) - 1900;
> > +     tm->tm_yday     = rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year);
> > +
> > +     /* Compute day of week */
> > +     rtc_tm_to_time(tm, &time);
> > +     rtc_time_to_tm(time, &calc_tm);
> > +     tm->tm_wday = calc_tm.tm_wday;
>
> Do you really need an accurate wday? There are no userspace applications
> that I know of that care and converting back and forth is quite
> expensive.
>

We probably don't, this RTC should only get used for very specific purposes,
and any userspace programs should be using a different RTC.

> > +
> > +     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 */
> > +     int wday = tm->tm_wday == 6 ? 0 : tm->tm_wday + 1;
>
> I'm not sure how useful it is to set an accurate wday when the RTC is
> not able to give it back.
>

This is actually needed. The RTC on the EC controls battery charging
schedules and thus needs to know the day of the week, and this is the
way to do this.

> > +     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 = devm_rtc_device_register(&pdev->dev,
> > +                                                       DRV_NAME,
> > +                                                       &wilco_ec_rtc_ops,
> > +                                                       THIS_MODULE);
>
> Please use devm_rtc_allocate_device() and set rtc->range_min and
> rtc->range_max before registering with rtc_register_device().
>

Thanks, will do this.

> > +     if (IS_ERR(rtc))
> > +             return PTR_ERR(rtc);
> > +
> > +     return 0;
> > +}
> > +
> > +static struct platform_driver wilco_ec_rtc_driver = {
> > +     .driver = {
> > +             .name = DRV_NAME,
> > +     },
> > +     .probe = wilco_ec_rtc_probe,
> > +};
> > +
> > +module_platform_driver(wilco_ec_rtc_driver);
> > +
> > +MODULE_ALIAS("platform:" DRV_NAME);
> > +MODULE_AUTHOR("Nick Crews <ncrews@google.com>");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("Wilco EC RTC driver");
> > --
> > 2.20.1.97.g81188d93c3-goog
> >
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Nick Crews
ncrews@google.com

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

* Re: [PATCH v2 2/9] platform/chrome: Add new driver for Wilco EC
  2019-01-14 22:03 ` [PATCH v2 2/9] platform/chrome: Add new driver for Wilco EC Nick Crews
@ 2019-01-15 18:10   ` Enric Balletbo Serra
  0 siblings, 0 replies; 17+ messages in thread
From: Enric Balletbo Serra @ 2019-01-15 18:10 UTC (permalink / raw)
  To: Nick Crews
  Cc: linux-kernel, Guenter Roeck, Simon Glass, dlaurie, Duncan Laurie,
	Enric Balletbo i Serra, Benson Leung

Hi Nick,

The patch looks pretty good to me, just some minor comments below ...

Missatge de Nick Crews <ncrews@google.com> del dia dl., 14 de gen.
2019 a les 23:05:
>
> From: Duncan Laurie <dlaurie@google.com>
>
> This EC is an incompatible variant of the typical Chrome OS embedded
> controller.  It uses the same low-level communication and a similar
> protocol with some significant differences.  The EC firmware does
> not support the same mailbox commands so it is not registered as a
> cros_ec device type.
>
> Signed-off-by: Duncan Laurie <dlaurie@google.com>
> Signed-off-by: Nick Crews <ncrews@google.com>
> ---
>
> Changes in v2:
> - Removed COMPILE_TEST from Kconfig because inb()/outb()
> won't work on anything but X86
> - Moved everything to wilco_ec/ subdirectory
> - Moved header file to include/platform_data/
> so could be used by future sub-drivers
> - Tweaked mailbox()
> - Removed duplicate EC_MAILBOX_DATA_SIZE defs
> - Removed some error messages / converted to dbg()
>
>  drivers/platform/chrome/Kconfig            |   4 +-
>  drivers/platform/chrome/Makefile           |   2 +
>  drivers/platform/chrome/wilco_ec/Kconfig   |  24 +++
>  drivers/platform/chrome/wilco_ec/Makefile  |   4 +
>  drivers/platform/chrome/wilco_ec/core.c    |  99 +++++++++
>  drivers/platform/chrome/wilco_ec/mailbox.c | 237 +++++++++++++++++++++
>  include/linux/platform_data/wilco-ec.h     | 138 ++++++++++++
>  7 files changed, 507 insertions(+), 1 deletion(-)
>  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/mailbox.c
>  create mode 100644 include/linux/platform_data/wilco-ec.h
>
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 16b1615958aa..bf889adfd4ef 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -49,6 +49,8 @@ config CHROMEOS_TBMC
>           To compile this driver as a module, choose M here: the
>           module will be called chromeos_tbmc.
>
> +source "drivers/platform/chrome/wilco_ec/Kconfig"
> +
>  config CROS_EC_CTL
>          tristate
>
> @@ -86,7 +88,7 @@ config CROS_EC_LPC
>
>  config CROS_EC_LPC_MEC
>         bool "ChromeOS Embedded Controller LPC Microchip EC (MEC) variant"
> -       depends on CROS_EC_LPC
> +       depends on CROS_EC_LPC || WILCO_EC
>         default n
>         help
>           If you say Y here, a variant LPC protocol for the Microchip EC
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index cd591bf872bb..7242ee2b13c5 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -13,3 +13,5 @@ cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC)        += cros_ec_lpc_mec.o
>  obj-$(CONFIG_CROS_EC_LPC)              += cros_ec_lpcs.o
>  obj-$(CONFIG_CROS_EC_PROTO)            += cros_ec_proto.o
>  obj-$(CONFIG_CROS_KBD_LED_BACKLIGHT)   += cros_kbd_led_backlight.o
> +
> +obj-$(CONFIG_WILCO_EC)                 += wilco_ec/
> diff --git a/drivers/platform/chrome/wilco_ec/Kconfig b/drivers/platform/chrome/wilco_ec/Kconfig
> new file mode 100644
> index 000000000000..f8e6c9e8c5cd
> --- /dev/null
> +++ b/drivers/platform/chrome/wilco_ec/Kconfig
> @@ -0,0 +1,24 @@
> +menuconfig WILCO_EC_PLATFORM
> +       bool "Platform support for Wilco Embedded Controller hardware"
> +       help
> +         Say Y here to get to see options for platform support for
> +         various Wilco EC features. This option alone does
> +         not add any kernel code.
> +
> +         If you say N, all options in this submenu will be skipped and disabled.
> +
> +if WILCO_EC_PLATFORM
> +
> +config WILCO_EC
> +       tristate "ChromeOS Wilco Embedded Controller"
> +       depends on ACPI && X86
> +       select CROS_EC_LPC_MEC
> +       help
> +         If you say Y here, you get support for talking to the ChromeOS
> +         Wilco EC over an eSPI bus. This uses a simple byte-level protocol
> +         with a checksum.
> +
> +         To compile this driver as a module, choose M here: the
> +         module will be called wilco_ec.
> +
> +endif # WILCO_EC_PLATFORM
> diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
> new file mode 100644
> index 000000000000..03b32301dc61
> --- /dev/null
> +++ b/drivers/platform/chrome/wilco_ec/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +wilco_ec-objs                          := core.o mailbox.o
> +obj-$(CONFIG_WILCO_EC)                 += wilco_ec.o
> diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
> new file mode 100644
> index 000000000000..2d1d8d3f9a94
> --- /dev/null
> +++ b/drivers/platform/chrome/wilco_ec/core.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Mailbox interface for Wilco Embedded Controller
> + *
> + * Copyright 2018 Google LLC
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/device.h>
> +#include <linux/ioport.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/wilco-ec.h>
> +#include <linux/platform_device.h>
> +
> +#include "../cros_ec_lpc_mec.h"
> +
> +static struct resource *wilco_get_resource(struct platform_device *pdev,
> +                                          int index)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct resource *res;
> +
> +       res = platform_get_resource(pdev, IORESOURCE_IO, index);
> +       if (!res) {
> +               dev_dbg(dev, "couldn't find IO resource %d\n", index);
> +               return NULL;
return res; ?

> +       }
> +
> +       if (!devm_request_region(dev, res->start, resource_size(res),
> +                                dev_name(dev)))
> +               return NULL;
> +
> +       return res;

return devm_request_region(dev, res->start,
resource_size(res),dev_name(dev))); ?

> +}
> +
> +static int wilco_ec_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct wilco_ec_device *ec;
> +       int ret;

This variable is not used, you can remove it.

> +
> +       ec = devm_kzalloc(dev, sizeof(*ec), GFP_KERNEL);
> +       if (!ec)
> +               return -ENOMEM;

nit: I like to see a blank line here

> +       platform_set_drvdata(pdev, ec);
> +       ec->dev = dev;
> +       mutex_init(&ec->mailbox_lock);
> +
> +       /* Largest data buffer size requirement is extended data response */
> +       ec->data_size = sizeof(struct wilco_ec_response) +
> +               EC_MAILBOX_DATA_SIZE_EXTENDED;
> +       ec->data_buffer = devm_kzalloc(dev, ec->data_size, GFP_KERNEL);
> +       if (!ec->data_buffer)
> +               return -ENOMEM;
> +
> +       /* Prepare access to IO regions provided by ACPI */
> +       ec->io_data = wilco_get_resource(pdev, 0);      /* Host Data */
> +       ec->io_command = wilco_get_resource(pdev, 1);   /* Host Command */
> +       ec->io_packet = wilco_get_resource(pdev, 2);    /* MEC EMI */
> +       if (!ec->io_data || !ec->io_command || !ec->io_packet)
> +               return -ENODEV;
> +
> +       /* Initialize cros_ec register interface for communication */
> +       cros_ec_lpc_mec_init(ec->io_packet->start,
> +                            ec->io_packet->start + EC_MAILBOX_DATA_SIZE);
> +
> +       return 0;
> +}
> +
> +static int wilco_ec_remove(struct platform_device *pdev)
> +{
> +       /* Teardown cros_ec interface */
> +       cros_ec_lpc_mec_destroy();
> +
> +       return 0;
> +}
> +
> +static const struct acpi_device_id wilco_ec_acpi_device_ids[] = {
> +       { "GOOG000C", 0 },
> +       { }
> +};
> +MODULE_DEVICE_TABLE(acpi, wilco_ec_acpi_device_ids);
> +
> +static struct platform_driver wilco_ec_driver = {
> +       .driver = {
> +               .name = "wilco_ec",
> +               .acpi_match_table = wilco_ec_acpi_device_ids,
> +       },
> +       .probe = wilco_ec_probe,
> +       .remove = wilco_ec_remove,
> +};
> +
> +module_platform_driver(wilco_ec_driver);
> +
> +MODULE_AUTHOR("Nick Crews <ncrews@google.com>");
> +MODULE_AUTHOR("Duncan Laurie <dlaurie@chromium.org>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("ChromeOS Wilco Embedded Controller driver");
> +MODULE_ALIAS("platform:wilco-ec");
> diff --git a/drivers/platform/chrome/wilco_ec/mailbox.c b/drivers/platform/chrome/wilco_ec/mailbox.c
> new file mode 100644
> index 000000000000..221de0d020cf
> --- /dev/null
> +++ b/drivers/platform/chrome/wilco_ec/mailbox.c
> @@ -0,0 +1,237 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Mailbox interface for Wilco Embedded Controller
> + *
> + * Copyright 2018 Google LLC
> + *
> + * The Wilco EC is similar to a typical ChromeOS embedded controller.
> + * It uses the same MEC based low-level communication and a similar
> + * protocol, but with some important differences.  The EC firmware does
> + * not support the same mailbox commands so it is not registered as a
> + * cros_ec device type.
> + *
> + * Most messages follow a standard format, but there are some exceptions
> + * and an interface is provided to do direct/raw transactions that do not
> + * make assumptions about byte placement.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/io.h>
> +#include <linux/platform_data/wilco-ec.h>
> +#include <linux/platform_device.h>
> +
> +#include "../cros_ec_lpc_mec.h"
> +
> +/* Version of mailbox interface */
> +#define EC_MAILBOX_VERSION             0
> +
> +/* Command to start mailbox transaction */
> +#define EC_MAILBOX_START_COMMAND       0xda
> +
> +/* Version of EC protocol */
> +#define EC_MAILBOX_PROTO_VERSION       3
> +
> +/* Number of header bytes to be counted as data bytes */
> +#define EC_MAILBOX_DATA_EXTRA          2
> +
> +/* Maximum timeout */
> +#define EC_MAILBOX_TIMEOUT             HZ
> +
> +/* EC response flags */
> +#define EC_CMDR_DATA           BIT(0)  /* Data ready for host to read */
> +#define EC_CMDR_PENDING                BIT(1)  /* Write pending to EC */
> +#define EC_CMDR_BUSY           BIT(2)  /* EC is busy processing a command */
> +#define EC_CMDR_CMD            BIT(3)  /* Last host write was a command */
> +
> +/**
> + * wilco_ec_response_timed_out() - Wait for EC response.
> + * @ec: EC device.
> + *
> + * Return: true if EC timed out, false if EC did not time out.
> + */
> +static bool wilco_ec_response_timed_out(struct wilco_ec_device *ec)
> +{
> +       unsigned long timeout = jiffies + EC_MAILBOX_TIMEOUT;
> +
> +       usleep_range(200, 300);
Is this sleep really needed here? The system will not respond in less
than 200us? What happens if you just remove this line?

> +       do {
> +               if (!(inb(ec->io_command->start) &
> +                     (EC_CMDR_PENDING | EC_CMDR_BUSY)))
> +                       return false;
> +               usleep_range(100, 200);
> +       } while (time_before(jiffies, timeout));
> +
> +       return true;
> +}
> +
> +/**
> + * wilco_ec_checksum() - Compute 8bit checksum over data range.

nit: 8-bit, to be coherent with the documentation with the previous patch

> + * @data: Data to checksum.
> + * @size: Number of bytes to checksum.
> + *
> + * Return: 8bit checksum of provided data.

nit: 8-bit, to be coherent with the documentation with the previous patch

> + */
> +static u8 wilco_ec_checksum(const void *data, size_t size)
> +{
> +       u8 *data_bytes = (u8 *)data;
> +       u8 checksum = 0;
> +       size_t i;
> +
> +       for (i = 0; i < size; i++)
> +               checksum += data_bytes[i];
> +
> +       return checksum;
> +}
> +
> +/**
> + * wilco_ec_prepare() - Prepare the request structure for the EC.
> + * @msg: EC message with request information.
> + * @rq: EC request structure to fill.
> + */
> +static void wilco_ec_prepare(struct wilco_ec_message *msg,
> +                            struct wilco_ec_request *rq)
> +{
> +       memset(rq, 0, sizeof(*rq));
> +
> +       /* Handle messages without trimming bytes from the request */
> +       if (msg->request_size && msg->flags & WILCO_EC_FLAG_RAW_REQUEST) {
> +               rq->reserved_raw = *(u8 *)msg->request_data;
> +               msg->request_size--;
> +               memmove(msg->request_data, msg->request_data + 1,
> +                       msg->request_size);
> +       }
> +
> +       /* Fill in request packet */
> +       rq->struct_version = EC_MAILBOX_PROTO_VERSION;
> +       rq->mailbox_id = msg->type;
> +       rq->mailbox_version = EC_MAILBOX_VERSION;
> +       rq->data_size = msg->request_size + EC_MAILBOX_DATA_EXTRA;
> +       rq->command = msg->command;
> +
> +       /* Checksum header and data */
> +       rq->checksum = wilco_ec_checksum(rq, sizeof(*rq));
> +       rq->checksum += wilco_ec_checksum(msg->request_data, msg->request_size);
> +       rq->checksum = -rq->checksum;
> +}
> +
> +/**
> + * wilco_ec_transfer() - Perform actual data transfer.
> + * @ec: EC device.
> + * @msg: EC message data for request and response.
> + * @rq: Filled in request structure
> + *
> + * Context: ec->mailbox_lock should be held while using this function.
> + * Return: number of bytes received or negative error code on failure.
> + */
> +int wilco_ec_transfer(struct wilco_ec_device *ec, struct wilco_ec_message *msg,
> +                     struct wilco_ec_request *rq)
> +{
> +       struct wilco_ec_response *rs;
> +       u8 checksum;
> +       u8 flag;
> +       size_t size;
> +
> +       /* Write request header, then data */
> +       cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, 0, sizeof(*rq), (u8 *)rq);
> +       cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, sizeof(*rq), msg->request_size,
> +                                msg->request_data);
> +
> +       /* Start the command */
> +       outb(EC_MAILBOX_START_COMMAND, ec->io_command->start);
> +
> +       /* Wait for it to complete */
> +       if (wilco_ec_response_timed_out(ec)) {
> +               dev_dbg(ec->dev, "response timed out\n");
> +               return -ETIMEDOUT;
> +       }
> +
> +       /* For some commands (eg shutdown) the EC will not respond, that's OK */
> +       if (msg->flags & WILCO_EC_FLAG_NO_RESPONSE) {
> +               dev_dbg(ec->dev, "EC does not respond to this command\n");
> +               return 0;
> +       }
> +
> +       /* Check result */
> +       flag = inb(ec->io_data->start);
> +       if (flag) {
> +               dev_dbg(ec->dev, "bad response: 0x%02x\n", flag);
> +               return -EIO;
> +       }
> +
> +       if (msg->flags & WILCO_EC_FLAG_EXTENDED_DATA)
> +               size = EC_MAILBOX_DATA_SIZE_EXTENDED;
> +       else
> +               size = EC_MAILBOX_DATA_SIZE;
> +
> +       /* Read back response */
> +       rs = ec->data_buffer;
> +       checksum = cros_ec_lpc_io_bytes_mec(MEC_IO_READ, 0,
> +                                           sizeof(*rs) + size, (u8 *)rs);
> +       if (checksum) {
> +               dev_dbg(ec->dev, "bad packet checksum 0x%02x\n", rs->checksum);
> +               return -EBADMSG;
> +       }
> +
> +       /* Check that the EC reported success */
> +       msg->result = rs->result;
> +       if (msg->result) {
> +               dev_dbg(ec->dev, "bad response: 0x%02x\n", msg->result);
> +               return -EBADMSG;
> +       }
> +
> +       /* Check the returned data size, skipping the header */
> +       if (rs->data_size != size) {
> +               dev_dbg(ec->dev, "unexpected packet size (%u != %zu)",
> +                       rs->data_size, size);
> +               return -EMSGSIZE;
> +       }
> +
> +       /* Skip 1 response data byte unless specified */
> +       size = (msg->flags & WILCO_EC_FLAG_RAW_RESPONSE) ? 0 : 1;
> +       if ((ssize_t) rs->data_size - size < msg->response_size) {
> +               dev_dbg(ec->dev, "response data too short (%zd < %zu)",
> +                       (ssize_t) rs->data_size - size, msg->response_size);
> +               return -EMSGSIZE;
> +       }
> +
> +       /* Ignore response data bytes as requested */
> +       memcpy(msg->response_data, rs->data + size, msg->response_size);
> +
> +       /* Return actual amount of data received */
> +       return msg->response_size;
> +}
> +
> +/**
> + * wilco_ec_mailbox() - Send EC request and receive EC response.
> + * @ec: EC device.
> + * @msg: EC message data for request and response.
> + *
> + * On entry msg->type, msg->flags, msg->command, msg->request_size,
> + * msg->response_size, and msg->request_data should all be filled in.
> + *
> + * On exit msg->result and msg->response_data will be filled.
> + *
> + * Return: number of bytes received or negative error code on failure.
> + */
> +int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg)
> +{
> +       struct wilco_ec_request *rq;
> +       int ret;
> +
> +       dev_dbg(ec->dev, "cmd=%02x type=%04x flags=%02x rslen=%zu rqlen=%zu\n",
> +               msg->command, msg->type, msg->flags, msg->response_size,
> +               msg->request_size);
> +
> +       /* Prepare request packet */
> +       rq = ec->data_buffer;
> +       wilco_ec_prepare(msg, rq);
> +
> +       mutex_lock(&ec->mailbox_lock);
> +       ret = wilco_ec_transfer(ec, msg, rq);
> +       mutex_unlock(&ec->mailbox_lock);
> +
> +       return ret;
> +
> +}
> +EXPORT_SYMBOL_GPL(wilco_ec_mailbox);
> diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
> new file mode 100644
> index 000000000000..5477b8802f81
> --- /dev/null
> +++ b/include/linux/platform_data/wilco-ec.h
> @@ -0,0 +1,138 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * ChromeOS Wilco Embedded Controller
> + *
> + * Copyright 2018 Google LLC
> + */
> +
> +#ifndef WILCO_EC_H
> +#define WILCO_EC_H
> +
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +
> +#define WILCO_EC_FLAG_NO_RESPONSE      BIT(0) /* EC does not respond */
> +#define WILCO_EC_FLAG_EXTENDED_DATA    BIT(1) /* EC returns 256 data bytes */
> +#define WILCO_EC_FLAG_RAW_REQUEST      BIT(2) /* Do not trim request data */
> +#define WILCO_EC_FLAG_RAW_RESPONSE     BIT(3) /* Do not trim response data */
> +#define WILCO_EC_FLAG_RAW              (WILCO_EC_FLAG_RAW_REQUEST | \
> +                                        WILCO_EC_FLAG_RAW_RESPONSE)
> +
> +/* Normal commands have a maximum 32 bytes of data */
> +#define EC_MAILBOX_DATA_SIZE           32
> +
> +/* Extended commands have 256 bytes of response data */
> +#define EC_MAILBOX_DATA_SIZE_EXTENDED  256
> +
> +/**
> + * struct wilco_ec_device - Wilco Embedded Controller handle.
> + * @dev: Device handle.
> + * @mailbox_lock: Mutex to ensure one mailbox command at a time.
> + * @io_command: I/O port for mailbox command.  Provided by ACPI.
> + * @io_data: I/O port for mailbox data.  Provided by ACPI.
> + * @io_packet: I/O port for mailbox packet data.  Provided by ACPI.
> + * @data_buffer: Buffer used for EC communication.  The same buffer
> + *               is used to hold the request and the response.
> + * @data_size: Size of the data buffer used for EC communication.
> + */
> +struct wilco_ec_device {
> +       struct device *dev;
> +       struct mutex mailbox_lock;
> +       struct resource *io_command;
> +       struct resource *io_data;
> +       struct resource *io_packet;
> +       void *data_buffer;
> +       size_t data_size;
> +};
> +
> +/**
> + * enum wilco_ec_msg_type - Message type to select a set of command codes.
> + * @WILCO_EC_MSG_LEGACY: Legacy EC messages for standard EC behavior.
> + * @WILCO_EC_MSG_PROPERTY: Get/Set/Sync EC controlled NVRAM property.
> + * @WILCO_EC_MSG_TELEMETRY: Telemetry data provided by the EC.
> + */
> +enum wilco_ec_msg_type {
> +       WILCO_EC_MSG_LEGACY = 0x00f0,
> +       WILCO_EC_MSG_PROPERTY = 0x00f2,
> +       WILCO_EC_MSG_TELEMETRY = 0x00f5,
> +};
> +
> +/**
> + * struct wilco_ec_message - Request and response message.
> + * @type: Mailbox message type.
> + * @flags: Message flags.
> + * @command: Mailbox command code.
> + * @result: Result code from the EC.  Non-zero indicates an error.
> + * @request_size: Number of bytes to send to the EC.
> + * @request_data: Buffer containing the request data.
> + * @response_size: Number of bytes expected from the EC.
> + *                 This is 32 by default and 256 if the flag
> + *                 is set for %WILCO_EC_FLAG_EXTENDED_DATA
> + * @response_data: Buffer containing the response data, should be
> + *                 response_size bytes and allocated by caller.
> + */
> +struct wilco_ec_message {
> +       enum wilco_ec_msg_type type;
> +       u8 flags;
> +       u8 command;
> +       u8 result;
> +       size_t request_size;
> +       void *request_data;
> +       size_t response_size;
> +       void *response_data;
> +};
> +
> +/**
> + * struct wilco_ec_request - Mailbox request message format.
> + * @struct_version: Should be %EC_MAILBOX_PROTO_VERSION
> + * @checksum: Sum of all bytes must be 0.
> + * @mailbox_id: Mailbox identifier, specifies the command set.
> + * @mailbox_version: Mailbox interface version %EC_MAILBOX_VERSION
> + * @reserved: Set to zero.
> + * @data_size: Length of request, data + last 2 bytes of the header.
> + * @command: Mailbox command code, unique for each mailbox_id set.
> + * @reserved_raw: Set to zero for most commands, but is used by
> + *                some command types and for raw commands.
> + */
> +struct wilco_ec_request {
> +       u8 struct_version;
> +       u8 checksum;
> +       u16 mailbox_id;
> +       u8 mailbox_version;
> +       u8 reserved;
> +       u16 data_size;
> +       u8 command;
> +       u8 reserved_raw;
> +} __packed;
> +
> +/**
> + * struct wilco_ec_response - Mailbox response message format.
> + * @struct_version: Should be %EC_MAILBOX_PROTO_VERSION
> + * @checksum: Sum of all bytes must be 0.
> + * @result: Result code from the EC.  Non-zero indicates an error.
> + * @data_size: Length of the response data buffer.
> + * @reserved: Set to zero.
> + * @mbox0: EC returned data at offset 0 is unused (always 0) so this byte
> + *         is treated as part of the header instead of the data.
> + * @data: Response data buffer.  Max size is %EC_MAILBOX_DATA_SIZE_EXTENDED.
> + */
> +struct wilco_ec_response {
> +       u8 struct_version;
> +       u8 checksum;
> +       u16 result;
> +       u16 data_size;
> +       u8 reserved[2];
> +       u8 mbox0;
> +       u8 data[0];
> +} __packed;
> +
> +/**
> + * wilco_ec_mailbox() - Send request to the EC and receive the response.
> + * @ec: Wilco EC device.
> + * @msg: Wilco EC message.
> + *
> + * Return: Number of bytes received or negative error code on failure.
> + */
> +int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg);
> +
> +#endif /* WILCO_EC_H */
> --
> 2.20.1.97.g81188d93c3-goog
>

Thanks,
 Enric

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

* Re: [PATCH v2 3/9] platform/chrome: Add sysfs attributes
  2019-01-14 22:03 ` [PATCH v2 3/9] platform/chrome: Add sysfs attributes Nick Crews
@ 2019-01-15 19:32   ` Enric Balletbo Serra
  0 siblings, 0 replies; 17+ messages in thread
From: Enric Balletbo Serra @ 2019-01-15 19:32 UTC (permalink / raw)
  To: Nick Crews
  Cc: linux-kernel, Guenter Roeck, Simon Glass, dlaurie, Duncan Laurie,
	Enric Balletbo i Serra, Benson Leung

Hi Nick,

Some few comments more ...

Missatge de Nick Crews <ncrews@google.com> del dia dl., 14 de gen.
2019 a les 23:07:
>
> From: Duncan Laurie <dlaurie@google.com>
>
> Add some sample sysfs attributes for the Wilco EC that show how
> the mailbox interface works. "Legacy" attributes are those that
> existed in the EC before it was adapted to ChromeOS.
>
> > cat /sys/bus/platform/devices/GOOG000C\:00/version
> Label        : 99.99.99
> SVN Revision : 738ed.99
> Model Number : 08;8
> Build Date   : 08/30/18
>
> Signed-off-by: Duncan Laurie <dlaurie@google.com>
> Signed-off-by: Nick Crews <ncrews@google.com>
> ---
>
> Changes in v2:
> - 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 in sysfs_init() and its caller
>
>  .../ABI/testing/sysfs-platform-wilcoec        | 26 ++++++
>  drivers/platform/chrome/wilco_ec/Makefile     |  2 +-
>  drivers/platform/chrome/wilco_ec/core.c       | 15 +++
>  drivers/platform/chrome/wilco_ec/legacy.c     | 91 +++++++++++++++++++
>  drivers/platform/chrome/wilco_ec/legacy.h     | 47 ++++++++++
>  drivers/platform/chrome/wilco_ec/sysfs.c      | 66 ++++++++++++++
>  include/linux/platform_data/wilco-ec.h        | 14 +++
>  7 files changed, 260 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-platform-wilcoec

I'd like to rename this to sysfs-platform-wilco-ec (like cros-ec)

>  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/sysfs.c
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-wilcoec b/Documentation/ABI/testing/sysfs-platform-wilcoec
> new file mode 100644
> index 000000000000..09552338c160
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-platform-wilcoec
> @@ -0,0 +1,26 @@
> +What:          /sys/bus/platform/devices/GOOG000C\:00/version
> +Date:          January 2019
> +KernelVersion: 4.19

That will be probably 5.1

> +Description:
> +               Display Wilco Embedded Controller version info
> +
> +               Output will be similar to the example below:
> +               Label        : 95.00.06
> +               SVN Revision : 5960a.06
> +               Model Number : 08;8
> +               Build Date   : 11/29/18
> +
> +               Although this contains multiple pieces of data in a single sysfs
> +               attribute (inconsistent with normal sysfs conventions),
> +               this format was chosen to be consistent with the version
> +               attribute in /sys/class/chomeos/<device>/version
> +

I don't buy the reason to be inconsistent. I don't think you need to
be consistent with something that is wrong and not following the
conventions. Past errors should be fixed :)

I'd like to see here single values in single attributes, this will
also be easier to parse from userspace point of view.

> +What:          /sys/bus/platform/devices/GOOG000C\:00/stealth_mode
> +Date:          January 2019
> +KernelVersion: 4.19
> +Description:
> +               Turn stealth_mode on or off on EC. Stealth mode means that the
> +               various LEDs, the LCD backlight, and onboard speakers are turned
> +               off and remain off even if activated from the off state.
> +               External monitors connected to the system are not affected.
> +               In addition Wireless devices are turned off.

Would be good to specify the valid values to write.

> diff --git a/drivers/platform/chrome/wilco_ec/Makefile b/drivers/platform/chrome/wilco_ec/Makefile
> index 03b32301dc61..7965a39e2e7a 100644
> --- a/drivers/platform/chrome/wilco_ec/Makefile
> +++ b/drivers/platform/chrome/wilco_ec/Makefile
> @@ -1,4 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>
> -wilco_ec-objs                          := core.o mailbox.o
> +wilco_ec-objs                          := core.o mailbox.o sysfs.o legacy.o
>  obj-$(CONFIG_WILCO_EC)                 += wilco_ec.o
> diff --git a/drivers/platform/chrome/wilco_ec/core.c b/drivers/platform/chrome/wilco_ec/core.c
> index 2d1d8d3f9a94..3193e8b2ec91 100644
> --- a/drivers/platform/chrome/wilco_ec/core.c
> +++ b/drivers/platform/chrome/wilco_ec/core.c
> @@ -64,11 +64,26 @@ static int wilco_ec_probe(struct platform_device *pdev)
>         cros_ec_lpc_mec_init(ec->io_packet->start,
>                              ec->io_packet->start + EC_MAILBOX_DATA_SIZE);
>
> +       /* Create sysfs attributes for userspace interaction */
> +       ret = wilco_ec_sysfs_init(ec);
> +       if (ret < 0) {
> +               dev_err(dev, "Failed to create sysfs attributes\n");
> +               goto destroy_lpc_mec;
> +       }
>         return 0;
> +
> +destroy_lpc_mec:
> +       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);
> +
> +       /* Remove sysfs attributes */
> +       wilco_ec_sysfs_remove(ec);
> +
>         /* Teardown cros_ec interface */
>         cros_ec_lpc_mec_destroy();
>
> diff --git a/drivers/platform/chrome/wilco_ec/legacy.c b/drivers/platform/chrome/wilco_ec/legacy.c
> new file mode 100644
> index 000000000000..1fdf94d22f57
> --- /dev/null
> +++ b/drivers/platform/chrome/wilco_ec/legacy.c
> @@ -0,0 +1,91 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Legacy (non-Chrome-specific) sysfs attributes for Wilco EC
> + *
> + * Copyright 2018 Google LLC
> + */
> +
> +#include <linux/ctype.h>
> +#include <linux/device.h>
> +#include <linux/platform_data/wilco-ec.h>
> +
> +#include "legacy.h"
> +
> +struct ec_info {
> +       u8 index;
> +       const char *label;
> +};
> +
> +static ssize_t wilco_ec_show_info(struct wilco_ec_device *ec, char *buf,
> +                                 ssize_t count, struct ec_info *info)
> +{
> +       char result[EC_INFO_SIZE];
> +       struct wilco_ec_message msg = {
> +               .type = WILCO_EC_MSG_LEGACY,
> +               .command = EC_COMMAND_EC_INFO,
> +               .request_data = &info->index,
> +               .request_size = sizeof(info->index),
> +               .response_data = result,
> +               .response_size = EC_INFO_SIZE,
> +       };
> +       int ret;
> +
> +       ret = wilco_ec_mailbox(ec, &msg);
> +       if (ret != EC_INFO_SIZE)
> +               return scnprintf(buf + count, PAGE_SIZE - count,
> +                                "%-12s : ERROR %d\n", info->label, ret);
> +
> +       return scnprintf(buf + count, PAGE_SIZE - count,
> +                        "%-12s : %s\n", info->label, result);
> +}
> +
> +ssize_t wilco_ec_version_show(struct device *dev, struct device_attribute *attr,
> +                             char *buf)
> +{
> +       struct wilco_ec_device *ec = dev_get_drvdata(dev);
> +       struct ec_info wilco_ec_info[] = {
> +               { 0, "Label" },
> +               { 1, "SVN Revision" },
> +               { 2, "Model Number" },
> +               { 3, "Build Date" },
> +               { 0xff, NULL },
> +       };
> +       struct ec_info *info = wilco_ec_info;
> +       ssize_t c = 0;
> +
> +       for (info = wilco_ec_info; info->label; info++)
> +               c += wilco_ec_show_info(ec, buf, c, info);
> +
> +       return c;
> +}
> +
> +ssize_t wilco_ec_stealth_mode_store(struct device *dev,
> +                                   struct device_attribute *attr,
> +                                   const char *buf, size_t count)
> +{
> +       struct wilco_ec_device *ec = dev_get_drvdata(dev);
> +       u8 param;
> +       struct wilco_ec_message msg = {
> +               .type = WILCO_EC_MSG_LEGACY,
> +               .command = EC_COMMAND_STEALTH_MODE,
> +               .request_data = &param,
> +               .request_size = sizeof(param),
> +       };
> +       int ret;
> +       bool enable;
> +
> +       ret = kstrtobool(buf, &enable);
> +       if (ret) {
> +               dev_err(dev, "Unable to parse '%s' to bool", buf);
> +               return ret;
> +       }
> +
> +       /* Invert input parameter, EC expects 0=on and 1=off */
> +       param = !enable;
> +
> +       ret = wilco_ec_mailbox(ec, &msg);
> +       if (ret < 0)
> +               return ret;
> +
> +       return count;
> +}
> diff --git a/drivers/platform/chrome/wilco_ec/legacy.h b/drivers/platform/chrome/wilco_ec/legacy.h
> new file mode 100644
> index 000000000000..5c857cb57fa9
> --- /dev/null
> +++ b/drivers/platform/chrome/wilco_ec/legacy.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Legacy (non-Chrome-specific) sysfs attributes for Wilco EC
> + *
> + * Copyright 2018 Google LLC
> + */
> +
> +#ifndef WILCO_EC_LEGACY_H
> +#define WILCO_EC_LEGACY_H
> +
> +#include <linux/device.h>
> +
> +#define EC_COMMAND_EC_INFO             0x38
> +#define EC_INFO_SIZE                   9
> +#define EC_COMMAND_STEALTH_MODE                0xfc
> +
> +/**
> + * wilco_ec_version_show() - Display Wilco Embedded Controller version info
> + *
> + * Output will be similar to the example below:
> + * Label        : 95.00.06
> + * SVN Revision : 5960a.06
> + * Model Number : 08;8
> + * Build Date   : 11/29/18
> + *
> + * Although this contains multiple pieces of data in a single sysfs attribute
> + * (inconsistent with normal sysfs conventions), this format was chosen to be
> + * consistent with the version attribute in ../cros_ec_sysfs.c
> + */
> +ssize_t wilco_ec_version_show(struct device *dev, struct device_attribute *attr,
> +                             char *buf);
> +
> +/**
> + * wilco_ec_stealth_mode_store() - Turn stealth_mode on or off on EC
> + * @dev: Device representing the EC
> + * @attr: The attribute in question
> + * @buf: Input buffer, should be parseable by kstrtobool(). Anything parsed to
> + *      True means enable stealth mode (turn off screen, etc)
> + * @count: Number of bytes in input buffer
> + *
> + * Return: Number of bytes consumed from input, negative error code on failure
> + */
> +ssize_t wilco_ec_stealth_mode_store(struct device *dev,
> +                                   struct device_attribute *attr,
> +                                   const char *buf, size_t count);
> +
> +#endif /* WILCO_EC_LEGACY_H */
> diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c b/drivers/platform/chrome/wilco_ec/sysfs.c
> new file mode 100644
> index 000000000000..e78a3ec3506b
> --- /dev/null
> +++ b/drivers/platform/chrome/wilco_ec/sysfs.c
> @@ -0,0 +1,66 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Sysfs attributes for Wilco Embedded Controller
> + *
> + * Copyright 2018 Google LLC
> + *
> + * The sysfs attributes appear under /sys/bus/platform/devices/GOOG000C\:00/
> + * To actually learn what each attribute does, read the corresponding _show() or
> + * _store() function source.
> + */
> +
> +#include <linux/ctype.h>
> +#include <linux/platform_data/wilco-ec.h>
> +#include <linux/platform_device.h>
> +#include <linux/sysfs.h>
> +
> +#include "legacy.h"
> +
> +#define WILCO_EC_ATTR_RO(_name)                                                \
> +__ATTR(_name, 0444, wilco_ec_##_name##_show, NULL)
> +
> +#define WILCO_EC_ATTR_WO(_name)                                                \
> +__ATTR(_name, 0200, NULL, wilco_ec_##_name##_store)
> +
> +#define WILCO_EC_ATTR_RW(_name)                                                \
> +__ATTR(_name, 0644, wilco_ec_##_name##_show, wilco_ec_##_name##_store)
> +

Why not use the already defined DEVICE_ATTR_* family instead?

> +/* Make top-level attributes, which will live inside GOOG000C:00/ */
> +static struct device_attribute version_attr = WILCO_EC_ATTR_RO(version);
> +static struct device_attribute stealth_attr = WILCO_EC_ATTR_WO(stealth_mode);
> +static struct attribute *wilco_ec_toplevel_attrs[] = {
> +       &version_attr.attr,
> +       &stealth_attr.attr,
> +       NULL
> +};
> +ATTRIBUTE_GROUPS(wilco_ec_toplevel);
> +
> +/**
> + * wilco_ec_sysfs_init() - Initialize the sysfs directories and attributes
> + * @dev: The device representing the EC
> + *
> + * Creates the sysfs directory structure and populates it with all attributes.
> + * If there is a problem it will clean up the entire filesystem.
> + *
> + * Return 0 on success, -ENOMEM on failure creating directories or attibutes.
> + */
> +int wilco_ec_sysfs_init(struct wilco_ec_device *ec)
> +{
> +       struct device *dev = ec->dev;
> +       int ret;
> +
> +       /* add the top-level attributes */
> +       ret = sysfs_create_groups(&dev->kobj, wilco_ec_toplevel_groups);
> +       if (ret)
> +               return -ENOMEM;

Do not overwrite the error, return ret;

> +
> +       return 0;
> +}
> +
> +void wilco_ec_sysfs_remove(struct wilco_ec_device *ec)
> +{
> +       struct device *dev = ec->dev;
> +
> +       /* go upwards through the directory structure */
> +       sysfs_remove_groups(&dev->kobj, wilco_ec_toplevel_groups);
> +}
> diff --git a/include/linux/platform_data/wilco-ec.h b/include/linux/platform_data/wilco-ec.h
> index 5477b8802f81..7c6ab6de7239 100644
> --- a/include/linux/platform_data/wilco-ec.h
> +++ b/include/linux/platform_data/wilco-ec.h
> @@ -135,4 +135,18 @@ struct wilco_ec_response {
>   */
>  int wilco_ec_mailbox(struct wilco_ec_device *ec, struct wilco_ec_message *msg);
>
> +/**
> + * wilco_ec_sysfs_init() - Create sysfs attributes.
> + * @ec: EC device.
> + *
> + * Return: 0 for success or negative error code on failure.
> + */
> +int wilco_ec_sysfs_init(struct wilco_ec_device *ec);
> +
> +/**
> + * wilco_ec_sysfs_remove() - Remove sysfs attributes.
> + * @ec: EC device.
> + */
> +void wilco_ec_sysfs_remove(struct wilco_ec_device *ec);
> +
>  #endif /* WILCO_EC_H */
> --
> 2.20.1.97.g81188d93c3-goog
>

Thanks,
  Enric

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

* Re: [PATCH v2 4/9] platform/chrome: Add support for raw commands in sysfs
  2019-01-14 22:03 ` [PATCH v2 4/9] platform/chrome: Add support for raw commands in sysfs Nick Crews
@ 2019-01-15 19:37   ` Enric Balletbo Serra
  2019-01-18 21:26     ` Nick Crews
  0 siblings, 1 reply; 17+ messages in thread
From: Enric Balletbo Serra @ 2019-01-15 19:37 UTC (permalink / raw)
  To: Nick Crews
  Cc: linux-kernel, Guenter Roeck, Simon Glass, dlaurie, Duncan Laurie,
	Enric Balletbo i Serra, Benson Leung

Hi Nick,

Missatge de Nick Crews <ncrews@google.com> del dia dl., 14 de gen.
2019 a les 23:07:
>
> From: Duncan Laurie <dlaurie@google.com>
>
> Add a sysfs attribute that allows sending raw commands to the EC.
> This is useful for development and debug but should not be enabled
> in a production environment.
>

Shouldn't you use debugfs for all this stuff instead?


> > echo 00 f0 38 00 03 00 > /sys/bus/platform/devices/GOOG000C\:00/raw
> > cat /sys/bus/platform/devices/GOOG000C\:00/raw
> 00 37 33 38 65 64 00...
>
> Signed-off-by: Duncan Laurie <dlaurie@google.com>
> Signed-off-by: Nick Crews <ncrews@google.com>
> ---
>
> Changes in v2:
> - Add sysfs documentation
> - rm duplicate EC_MAILBOX_DATA_SIZE defs
> - Make docstrings follow kernel style
> - Fix tags in commit msg
> - Reading raw now includes ASCII translation
>
>  .../ABI/testing/sysfs-platform-wilcoec        | 21 ++++
>  drivers/platform/chrome/wilco_ec/Kconfig      |  9 ++
>  drivers/platform/chrome/wilco_ec/legacy.c     | 99 +++++++++++++++++++
>  drivers/platform/chrome/wilco_ec/legacy.h     | 46 +++++++++
>  drivers/platform/chrome/wilco_ec/sysfs.c      |  6 ++
>  5 files changed, 181 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-wilcoec b/Documentation/ABI/testing/sysfs-platform-wilcoec
> index 09552338c160..cac2cf11835f 100644
> --- a/Documentation/ABI/testing/sysfs-platform-wilcoec
> +++ b/Documentation/ABI/testing/sysfs-platform-wilcoec
> @@ -24,3 +24,24 @@ Description:
>                 off and remain off even if activated from the off state.
>                 External monitors connected to the system are not affected.
>                 In addition Wireless devices are turned off.
> +
> +What:          /sys/bus/platform/devices/GOOG000C\:00/raw
> +Date:          January 2019
> +KernelVersion: 4.19
> +Description:
> +               Write and read raw mailbox commands to the EC.
> +
> +               For writing:
> +               Bytes 0-1 indicate the message type:
> +                       00 F0 = Execute Legacy Command
> +                       00 F2 = Read/Write NVRAM Property
> +               Byte 2 provides the command code
> +               Bytes 3+ consist of the data passed in the request
> +
> +               Example:
> +               // Request EC info type 3 (EC firmware build date)
> +               $ echo 00 f0 38 00 03 00 > raw
> +               // View the result. The decoded ASCII result "12/21/18" is
> +               // included after the raw hex.
> +               $ cat raw
> +               00 31 32 2f 32 31 2f 31 38 00 38 00 01 00 2f 00  .12/21/18.8...
> diff --git a/drivers/platform/chrome/wilco_ec/Kconfig b/drivers/platform/chrome/wilco_ec/Kconfig
> index f8e6c9e8c5cd..0bd84c98b79b 100644
> --- a/drivers/platform/chrome/wilco_ec/Kconfig
> +++ b/drivers/platform/chrome/wilco_ec/Kconfig
> @@ -21,4 +21,13 @@ config WILCO_EC
>           To compile this driver as a module, choose M here: the
>           module will be called wilco_ec.
>
> +config WILCO_EC_SYSFS_RAW
> +       bool "Enable raw access to EC via sysfs"
> +       depends on WILCO_EC
> +       help
> +         If you say Y here, you get support for sending raw commands to
> +         the Wilco EC via sysfs.  These commands do not do any byte
> +         manipulation and allow for testing arbitrary commands.  This
> +         interface is intended for debug only and is disabled by default.
> +
>  endif # WILCO_EC_PLATFORM
> diff --git a/drivers/platform/chrome/wilco_ec/legacy.c b/drivers/platform/chrome/wilco_ec/legacy.c
> index 1fdf94d22f57..79bac5dccb39 100644
> --- a/drivers/platform/chrome/wilco_ec/legacy.c
> +++ b/drivers/platform/chrome/wilco_ec/legacy.c
> @@ -11,6 +11,105 @@
>
>  #include "legacy.h"
>
> +#ifdef CONFIG_WILCO_EC_SYSFS_RAW
> +
> +/* Raw data buffer, large enough to hold extended responses */
> +static size_t raw_response_size;
> +static u8 raw_response_data[EC_MAILBOX_DATA_SIZE_EXTENDED];
> +
> +ssize_t wilco_ec_raw_store(struct device *dev, struct device_attribute *attr,
> +                          const char *buf, size_t count)
> +{
> +       struct wilco_ec_device *ec = dev_get_drvdata(dev);
> +       struct wilco_ec_message msg;
> +       u8 raw_request_data[EC_MAILBOX_DATA_SIZE];
> +       int in_offset = 0;
> +       int out_offset = 0;
> +       int ret;
> +
> +       while (in_offset < count) {
> +               char word_buf[EC_MAILBOX_DATA_SIZE];
> +               u8 byte;
> +               int start_offset = in_offset;
> +               int end_offset;
> +
> +               /* Find the start of the byte */
> +               while (buf[start_offset] && isspace(buf[start_offset]))
> +                       start_offset++;
> +               if (!buf[start_offset])
> +                       break;
> +
> +               /* Find the start of the next byte, if any */
> +               end_offset = start_offset;
> +               while (buf[end_offset] && !isspace(buf[end_offset]))
> +                       end_offset++;
> +               if (start_offset > count || end_offset > count)
> +                       break;
> +               if (start_offset > EC_MAILBOX_DATA_SIZE ||
> +                   end_offset > EC_MAILBOX_DATA_SIZE)
> +                       break;
> +
> +               /* Copy to a new NULL terminated string */
> +               memcpy(word_buf, buf + start_offset, end_offset - start_offset);
> +               word_buf[end_offset - start_offset] = '\0';
> +
> +               /* Convert from hex string */
> +               ret = kstrtou8(word_buf, 16, &byte);
> +               if (ret)
> +                       break;
> +
> +               /* Fill this byte into the request buffer */
> +               raw_request_data[out_offset++] = byte;
> +               if (out_offset >= EC_MAILBOX_DATA_SIZE)
> +                       break;
> +
> +               in_offset = end_offset;
> +       }
> +       if (out_offset == 0)
> +               return -EINVAL;
> +
> +       /* Clear response data buffer */
> +       memset(raw_response_data, '\0', EC_MAILBOX_DATA_SIZE_EXTENDED);
> +
> +       msg.type = raw_request_data[0] << 8 | raw_request_data[1];
> +       msg.flags = WILCO_EC_FLAG_RAW;
> +       msg.command = raw_request_data[2];
> +       msg.request_data = raw_request_data + 3;
> +       msg.request_size = out_offset - 3;
> +       msg.response_data = raw_response_data;
> +       msg.response_size = EC_MAILBOX_DATA_SIZE;
> +
> +       /* Telemetry commands use extended response data */
> +       if (msg.type == WILCO_EC_MSG_TELEMETRY) {
> +               msg.flags |= WILCO_EC_FLAG_EXTENDED_DATA;
> +               msg.response_size = EC_MAILBOX_DATA_SIZE_EXTENDED;
> +       }
> +
> +       ret = wilco_ec_mailbox(ec, &msg);
> +       if (ret < 0)
> +               return ret;
> +       raw_response_size = ret;
> +       return count;
> +}
> +
> +ssize_t wilco_ec_raw_show(struct device *dev, struct device_attribute *attr,
> +                         char *buf)
> +{
> +       ssize_t count = 0;
> +
> +       if (raw_response_size) {
> +               count = hex_dump_to_buffer(raw_response_data, raw_response_size,
> +                                          16, 1, buf, PAGE_SIZE, true);
> +
> +               /* Only return response the first time it is read */
> +               raw_response_size = 0;
> +       }
> +
> +       return count;
> +}
> +
> +#endif /* CONFIG_WILCO_EC_SYSFS_RAW */
> +
>  struct ec_info {
>         u8 index;
>         const char *label;
> diff --git a/drivers/platform/chrome/wilco_ec/legacy.h b/drivers/platform/chrome/wilco_ec/legacy.h
> index 5c857cb57fa9..340787d25d44 100644
> --- a/drivers/platform/chrome/wilco_ec/legacy.h
> +++ b/drivers/platform/chrome/wilco_ec/legacy.h
> @@ -14,6 +14,52 @@
>  #define EC_INFO_SIZE                   9
>  #define EC_COMMAND_STEALTH_MODE                0xfc
>
> +#ifdef CONFIG_WILCO_EC_SYSFS_RAW
> +
> +/**
> + * wilco_ec_raw_store() - Write a raw command to EC, store result to view later
> + * @dev: Device representing the EC
> + * @attr: The attribute in question
> + * @buf: Input buffer, format explained below
> + * @count: Number of bytes in input buffer
> + *
> + * Bytes 0-1 indicate the message type:
> + *  00 F0 = Execute Legacy Command
> + *  00 F2 = Read/Write NVRAM Property
> + * Byte 2 provides the command code
> + * Bytes 3+ consist of the data passed in the request
> + *
> + * example: read the EC info type 3 (EC firmware build date):
> + *  # echo 00 f0 38 00 03 00 > raw
> + *
> + * After calling this function, read the result by using raw_show()
> + *
> + * Return: Number of bytes consumed from input, negative error code on failure
> + */
> +ssize_t wilco_ec_raw_store(struct device *dev, struct device_attribute *attr,
> +                          const char *buf, size_t count);
> +
> +/**
> + * wilco_ec_raw_show() - Show result from previous call to raw_store()
> + * @dev: Device representing the EC
> + * @attr: The attribute in question
> + * @buf: Output buffer to be filled
> + *
> + * Example usage:
> + *     // Call raw_store(), read EC info type 3 (EC firmware build date)
> + *     # echo 00 f0 38 00 03 00 > raw
> + *     // Call this function, view the result. The decoded ASCII result
> + *     // "12/21/18" is included after the raw hex.
> + *     # cat raw
> + *     00 31 32 2f 32 31 2f 31 38 00 38 00 01 00 2f 00  .12/21/18.8.../.
> + *
> + * Return: Number of bytes written to output, negative error code on failure
> + */
> +ssize_t wilco_ec_raw_show(struct device *dev, struct device_attribute *attr,
> +                         char *buf);
> +
> +#endif /* CONFIG_WILCO_EC_SYSFS_RAW */
> +
>  /**
>   * wilco_ec_version_show() - Display Wilco Embedded Controller version info
>   *
> diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c b/drivers/platform/chrome/wilco_ec/sysfs.c
> index e78a3ec3506b..0611a73fcdce 100644
> --- a/drivers/platform/chrome/wilco_ec/sysfs.c
> +++ b/drivers/platform/chrome/wilco_ec/sysfs.c
> @@ -28,9 +28,15 @@ __ATTR(_name, 0644, wilco_ec_##_name##_show, wilco_ec_##_name##_store)
>  /* Make top-level attributes, which will live inside GOOG000C:00/ */
>  static struct device_attribute version_attr = WILCO_EC_ATTR_RO(version);
>  static struct device_attribute stealth_attr = WILCO_EC_ATTR_WO(stealth_mode);
> +#ifdef CONFIG_WILCO_EC_SYSFS_RAW
> +static struct device_attribute raw_attr = WILCO_EC_ATTR_RW(raw);
> +#endif
>  static struct attribute *wilco_ec_toplevel_attrs[] = {
>         &version_attr.attr,
>         &stealth_attr.attr,
> +#ifdef CONFIG_WILCO_EC_SYSFS_RAW
> +       &raw_attr.attr,
> +#endif
>         NULL
>  };
>  ATTRIBUTE_GROUPS(wilco_ec_toplevel);
> --
> 2.20.1.97.g81188d93c3-goog
>

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

* Re: [PATCH v2 5/9] platform/chrome: rtc: Add RTC driver for Wilco EC
  2019-01-14 22:25   ` Alexandre Belloni
  2019-01-15  0:30     ` Nick Crews
@ 2019-01-18 21:24     ` Nick Crews
  1 sibling, 0 replies; 17+ messages in thread
From: Nick Crews @ 2019-01-18 21:24 UTC (permalink / raw)
  To: Alexandre Belloni
  Cc: linux-kernel, groeck, sjg, dlaurie, Duncan Laurie, linux-rtc,
	Enric Balletbo i Serra, Alessandro Zummo, Benson Leung

Hi Alexandre, thanks for taking the time to review this. I've responded to your
comments inline below. I'll send out a new version of this patch soon.

On Mon, Jan 14, 2019 at 3:26 PM Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> Hello,
>
> On 14/01/2019 15:03:52-0700, Nick Crews wrote:
> > diff --git a/drivers/rtc/rtc-wilco-ec.c b/drivers/rtc/rtc-wilco-ec.c
> > new file mode 100644
> > index 000000000000..d8ed791da82d
> > --- /dev/null
> > +++ b/drivers/rtc/rtc-wilco-ec.c
> > @@ -0,0 +1,178 @@
> > +// 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 DRV_NAME "rtc-wilco-ec"
>
> Please get rid of that define.
>

Done.

> > +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),
> > +     };
> > +     struct rtc_time calc_tm;
> > +     unsigned long time;
> > +     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;
> > +     /*
> > +      * The RTC stores the month value as 1-12 but the kernel expects 0-11,
> > +      * so ensure invalid/zero month value from RTC is not converted to -1.
>
> I'm pretty sure this is not necessary and will certainly make the core
> think the date is valid when it has obviously been corrupted.

Done, I'll just use rtc.month - 1 always.

>
> > +      */
> > +     tm->tm_mon      = rtc.month ? rtc.month - 1 : 0;
> > +     tm->tm_year     = rtc.year + (rtc.century * 100) - 1900;
> > +     tm->tm_yday     = rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year);
> > +
> > +     /* Compute day of week */
> > +     rtc_tm_to_time(tm, &time);
> > +     rtc_time_to_tm(time, &calc_tm);
> > +     tm->tm_wday = calc_tm.tm_wday;
>
> Do you really need an accurate wday? There are no userspace applications
> that I know of that care and converting back and forth is quite
> expensive.

No, I'm not sure we do need this. Removing it.

>
> > +
> > +     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 */
> > +     int wday = tm->tm_wday == 6 ? 0 : tm->tm_wday + 1;
>
> I'm not sure how useful it is to set an accurate wday when the RTC is
> not able to give it back.

This is actually needed. The RTC controls battery charging schedule,
which varies by the day of the week, and it requires this to be set.

>
> > +     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 = devm_rtc_device_register(&pdev->dev,
> > +                                                       DRV_NAME,
> > +                                                       &wilco_ec_rtc_ops,
> > +                                                       THIS_MODULE);
>
> Please use devm_rtc_allocate_device() and set rtc->range_min and
> rtc->range_max before registering with rtc_register_device().

Done.

>
> > +     if (IS_ERR(rtc))
> > +             return PTR_ERR(rtc);
> > +
> > +     return 0;
> > +}
> > +
> > +static struct platform_driver wilco_ec_rtc_driver = {
> > +     .driver = {
> > +             .name = DRV_NAME,
> > +     },
> > +     .probe = wilco_ec_rtc_probe,
> > +};
> > +
> > +module_platform_driver(wilco_ec_rtc_driver);
> > +
> > +MODULE_ALIAS("platform:" DRV_NAME);
> > +MODULE_AUTHOR("Nick Crews <ncrews@google.com>");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("Wilco EC RTC driver");
> > --
> > 2.20.1.97.g81188d93c3-goog
> >
>
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Cheers,
Nick Crews

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

* Re: [PATCH v2 4/9] platform/chrome: Add support for raw commands in sysfs
  2019-01-15 19:37   ` Enric Balletbo Serra
@ 2019-01-18 21:26     ` Nick Crews
  0 siblings, 0 replies; 17+ messages in thread
From: Nick Crews @ 2019-01-18 21:26 UTC (permalink / raw)
  To: Enric Balletbo Serra
  Cc: linux-kernel, Guenter Roeck, Simon Glass, dlaurie, Duncan Laurie,
	Enric Balletbo i Serra, Benson Leung

Hi Enric, thanks for the comments. I'll send out a new version soon,
with this moved to debugfs.

On Tue, Jan 15, 2019 at 12:37 PM Enric Balletbo Serra
<eballetbo@gmail.com> wrote:
>
> Hi Nick,
>
> Missatge de Nick Crews <ncrews@google.com> del dia dl., 14 de gen.
> 2019 a les 23:07:
> >
> > From: Duncan Laurie <dlaurie@google.com>
> >
> > Add a sysfs attribute that allows sending raw commands to the EC.
> > This is useful for development and debug but should not be enabled
> > in a production environment.
> >
>
> Shouldn't you use debugfs for all this stuff instead?

Yes, I probably should. I've moved this all to there in the newest version.

>
>
> > > echo 00 f0 38 00 03 00 > /sys/bus/platform/devices/GOOG000C\:00/raw
> > > cat /sys/bus/platform/devices/GOOG000C\:00/raw
> > 00 37 33 38 65 64 00...
> >
> > Signed-off-by: Duncan Laurie <dlaurie@google.com>
> > Signed-off-by: Nick Crews <ncrews@google.com>
> > ---
> >
> > Changes in v2:
> > - Add sysfs documentation
> > - rm duplicate EC_MAILBOX_DATA_SIZE defs
> > - Make docstrings follow kernel style
> > - Fix tags in commit msg
> > - Reading raw now includes ASCII translation
> >
> >  .../ABI/testing/sysfs-platform-wilcoec        | 21 ++++
> >  drivers/platform/chrome/wilco_ec/Kconfig      |  9 ++
> >  drivers/platform/chrome/wilco_ec/legacy.c     | 99 +++++++++++++++++++
> >  drivers/platform/chrome/wilco_ec/legacy.h     | 46 +++++++++
> >  drivers/platform/chrome/wilco_ec/sysfs.c      |  6 ++
> >  5 files changed, 181 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-platform-wilcoec b/Documentation/ABI/testing/sysfs-platform-wilcoec
> > index 09552338c160..cac2cf11835f 100644
> > --- a/Documentation/ABI/testing/sysfs-platform-wilcoec
> > +++ b/Documentation/ABI/testing/sysfs-platform-wilcoec
> > @@ -24,3 +24,24 @@ Description:
> >                 off and remain off even if activated from the off state.
> >                 External monitors connected to the system are not affected.
> >                 In addition Wireless devices are turned off.
> > +
> > +What:          /sys/bus/platform/devices/GOOG000C\:00/raw
> > +Date:          January 2019
> > +KernelVersion: 4.19
> > +Description:
> > +               Write and read raw mailbox commands to the EC.
> > +
> > +               For writing:
> > +               Bytes 0-1 indicate the message type:
> > +                       00 F0 = Execute Legacy Command
> > +                       00 F2 = Read/Write NVRAM Property
> > +               Byte 2 provides the command code
> > +               Bytes 3+ consist of the data passed in the request
> > +
> > +               Example:
> > +               // Request EC info type 3 (EC firmware build date)
> > +               $ echo 00 f0 38 00 03 00 > raw
> > +               // View the result. The decoded ASCII result "12/21/18" is
> > +               // included after the raw hex.
> > +               $ cat raw
> > +               00 31 32 2f 32 31 2f 31 38 00 38 00 01 00 2f 00  .12/21/18.8...
> > diff --git a/drivers/platform/chrome/wilco_ec/Kconfig b/drivers/platform/chrome/wilco_ec/Kconfig
> > index f8e6c9e8c5cd..0bd84c98b79b 100644
> > --- a/drivers/platform/chrome/wilco_ec/Kconfig
> > +++ b/drivers/platform/chrome/wilco_ec/Kconfig
> > @@ -21,4 +21,13 @@ config WILCO_EC
> >           To compile this driver as a module, choose M here: the
> >           module will be called wilco_ec.
> >
> > +config WILCO_EC_SYSFS_RAW
> > +       bool "Enable raw access to EC via sysfs"
> > +       depends on WILCO_EC
> > +       help
> > +         If you say Y here, you get support for sending raw commands to
> > +         the Wilco EC via sysfs.  These commands do not do any byte
> > +         manipulation and allow for testing arbitrary commands.  This
> > +         interface is intended for debug only and is disabled by default.
> > +
> >  endif # WILCO_EC_PLATFORM
> > diff --git a/drivers/platform/chrome/wilco_ec/legacy.c b/drivers/platform/chrome/wilco_ec/legacy.c
> > index 1fdf94d22f57..79bac5dccb39 100644
> > --- a/drivers/platform/chrome/wilco_ec/legacy.c
> > +++ b/drivers/platform/chrome/wilco_ec/legacy.c
> > @@ -11,6 +11,105 @@
> >
> >  #include "legacy.h"
> >
> > +#ifdef CONFIG_WILCO_EC_SYSFS_RAW
> > +
> > +/* Raw data buffer, large enough to hold extended responses */
> > +static size_t raw_response_size;
> > +static u8 raw_response_data[EC_MAILBOX_DATA_SIZE_EXTENDED];
> > +
> > +ssize_t wilco_ec_raw_store(struct device *dev, struct device_attribute *attr,
> > +                          const char *buf, size_t count)
> > +{
> > +       struct wilco_ec_device *ec = dev_get_drvdata(dev);
> > +       struct wilco_ec_message msg;
> > +       u8 raw_request_data[EC_MAILBOX_DATA_SIZE];
> > +       int in_offset = 0;
> > +       int out_offset = 0;
> > +       int ret;
> > +
> > +       while (in_offset < count) {
> > +               char word_buf[EC_MAILBOX_DATA_SIZE];
> > +               u8 byte;
> > +               int start_offset = in_offset;
> > +               int end_offset;
> > +
> > +               /* Find the start of the byte */
> > +               while (buf[start_offset] && isspace(buf[start_offset]))
> > +                       start_offset++;
> > +               if (!buf[start_offset])
> > +                       break;
> > +
> > +               /* Find the start of the next byte, if any */
> > +               end_offset = start_offset;
> > +               while (buf[end_offset] && !isspace(buf[end_offset]))
> > +                       end_offset++;
> > +               if (start_offset > count || end_offset > count)
> > +                       break;
> > +               if (start_offset > EC_MAILBOX_DATA_SIZE ||
> > +                   end_offset > EC_MAILBOX_DATA_SIZE)
> > +                       break;
> > +
> > +               /* Copy to a new NULL terminated string */
> > +               memcpy(word_buf, buf + start_offset, end_offset - start_offset);
> > +               word_buf[end_offset - start_offset] = '\0';
> > +
> > +               /* Convert from hex string */
> > +               ret = kstrtou8(word_buf, 16, &byte);
> > +               if (ret)
> > +                       break;
> > +
> > +               /* Fill this byte into the request buffer */
> > +               raw_request_data[out_offset++] = byte;
> > +               if (out_offset >= EC_MAILBOX_DATA_SIZE)
> > +                       break;
> > +
> > +               in_offset = end_offset;
> > +       }
> > +       if (out_offset == 0)
> > +               return -EINVAL;
> > +
> > +       /* Clear response data buffer */
> > +       memset(raw_response_data, '\0', EC_MAILBOX_DATA_SIZE_EXTENDED);
> > +
> > +       msg.type = raw_request_data[0] << 8 | raw_request_data[1];
> > +       msg.flags = WILCO_EC_FLAG_RAW;
> > +       msg.command = raw_request_data[2];
> > +       msg.request_data = raw_request_data + 3;
> > +       msg.request_size = out_offset - 3;
> > +       msg.response_data = raw_response_data;
> > +       msg.response_size = EC_MAILBOX_DATA_SIZE;
> > +
> > +       /* Telemetry commands use extended response data */
> > +       if (msg.type == WILCO_EC_MSG_TELEMETRY) {
> > +               msg.flags |= WILCO_EC_FLAG_EXTENDED_DATA;
> > +               msg.response_size = EC_MAILBOX_DATA_SIZE_EXTENDED;
> > +       }
> > +
> > +       ret = wilco_ec_mailbox(ec, &msg);
> > +       if (ret < 0)
> > +               return ret;
> > +       raw_response_size = ret;
> > +       return count;
> > +}
> > +
> > +ssize_t wilco_ec_raw_show(struct device *dev, struct device_attribute *attr,
> > +                         char *buf)
> > +{
> > +       ssize_t count = 0;
> > +
> > +       if (raw_response_size) {
> > +               count = hex_dump_to_buffer(raw_response_data, raw_response_size,
> > +                                          16, 1, buf, PAGE_SIZE, true);
> > +
> > +               /* Only return response the first time it is read */
> > +               raw_response_size = 0;
> > +       }
> > +
> > +       return count;
> > +}
> > +
> > +#endif /* CONFIG_WILCO_EC_SYSFS_RAW */
> > +
> >  struct ec_info {
> >         u8 index;
> >         const char *label;
> > diff --git a/drivers/platform/chrome/wilco_ec/legacy.h b/drivers/platform/chrome/wilco_ec/legacy.h
> > index 5c857cb57fa9..340787d25d44 100644
> > --- a/drivers/platform/chrome/wilco_ec/legacy.h
> > +++ b/drivers/platform/chrome/wilco_ec/legacy.h
> > @@ -14,6 +14,52 @@
> >  #define EC_INFO_SIZE                   9
> >  #define EC_COMMAND_STEALTH_MODE                0xfc
> >
> > +#ifdef CONFIG_WILCO_EC_SYSFS_RAW
> > +
> > +/**
> > + * wilco_ec_raw_store() - Write a raw command to EC, store result to view later
> > + * @dev: Device representing the EC
> > + * @attr: The attribute in question
> > + * @buf: Input buffer, format explained below
> > + * @count: Number of bytes in input buffer
> > + *
> > + * Bytes 0-1 indicate the message type:
> > + *  00 F0 = Execute Legacy Command
> > + *  00 F2 = Read/Write NVRAM Property
> > + * Byte 2 provides the command code
> > + * Bytes 3+ consist of the data passed in the request
> > + *
> > + * example: read the EC info type 3 (EC firmware build date):
> > + *  # echo 00 f0 38 00 03 00 > raw
> > + *
> > + * After calling this function, read the result by using raw_show()
> > + *
> > + * Return: Number of bytes consumed from input, negative error code on failure
> > + */
> > +ssize_t wilco_ec_raw_store(struct device *dev, struct device_attribute *attr,
> > +                          const char *buf, size_t count);
> > +
> > +/**
> > + * wilco_ec_raw_show() - Show result from previous call to raw_store()
> > + * @dev: Device representing the EC
> > + * @attr: The attribute in question
> > + * @buf: Output buffer to be filled
> > + *
> > + * Example usage:
> > + *     // Call raw_store(), read EC info type 3 (EC firmware build date)
> > + *     # echo 00 f0 38 00 03 00 > raw
> > + *     // Call this function, view the result. The decoded ASCII result
> > + *     // "12/21/18" is included after the raw hex.
> > + *     # cat raw
> > + *     00 31 32 2f 32 31 2f 31 38 00 38 00 01 00 2f 00  .12/21/18.8.../.
> > + *
> > + * Return: Number of bytes written to output, negative error code on failure
> > + */
> > +ssize_t wilco_ec_raw_show(struct device *dev, struct device_attribute *attr,
> > +                         char *buf);
> > +
> > +#endif /* CONFIG_WILCO_EC_SYSFS_RAW */
> > +
> >  /**
> >   * wilco_ec_version_show() - Display Wilco Embedded Controller version info
> >   *
> > diff --git a/drivers/platform/chrome/wilco_ec/sysfs.c b/drivers/platform/chrome/wilco_ec/sysfs.c
> > index e78a3ec3506b..0611a73fcdce 100644
> > --- a/drivers/platform/chrome/wilco_ec/sysfs.c
> > +++ b/drivers/platform/chrome/wilco_ec/sysfs.c
> > @@ -28,9 +28,15 @@ __ATTR(_name, 0644, wilco_ec_##_name##_show, wilco_ec_##_name##_store)
> >  /* Make top-level attributes, which will live inside GOOG000C:00/ */
> >  static struct device_attribute version_attr = WILCO_EC_ATTR_RO(version);
> >  static struct device_attribute stealth_attr = WILCO_EC_ATTR_WO(stealth_mode);
> > +#ifdef CONFIG_WILCO_EC_SYSFS_RAW
> > +static struct device_attribute raw_attr = WILCO_EC_ATTR_RW(raw);
> > +#endif
> >  static struct attribute *wilco_ec_toplevel_attrs[] = {
> >         &version_attr.attr,
> >         &stealth_attr.attr,
> > +#ifdef CONFIG_WILCO_EC_SYSFS_RAW
> > +       &raw_attr.attr,
> > +#endif
> >         NULL
> >  };
> >  ATTRIBUTE_GROUPS(wilco_ec_toplevel);
> > --
> > 2.20.1.97.g81188d93c3-goog
> >

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

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

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-14 22:03 [PATCH v2 0/9] platform/chrome: rtc: Add support for Wilco EC Nick Crews
2019-01-14 22:03 ` [PATCH v2 1/9] platform/chrome: Remove cros_ec dependency in lpc_mec Nick Crews
2019-01-14 22:03 ` [PATCH v2 2/9] platform/chrome: Add new driver for Wilco EC Nick Crews
2019-01-15 18:10   ` Enric Balletbo Serra
2019-01-14 22:03 ` [PATCH v2 3/9] platform/chrome: Add sysfs attributes Nick Crews
2019-01-15 19:32   ` Enric Balletbo Serra
2019-01-14 22:03 ` [PATCH v2 4/9] platform/chrome: Add support for raw commands in sysfs Nick Crews
2019-01-15 19:37   ` Enric Balletbo Serra
2019-01-18 21:26     ` Nick Crews
2019-01-14 22:03 ` [PATCH v2 5/9] platform/chrome: rtc: Add RTC driver for Wilco EC Nick Crews
2019-01-14 22:25   ` Alexandre Belloni
2019-01-15  0:30     ` Nick Crews
2019-01-18 21:24     ` Nick Crews
2019-01-14 22:03 ` [PATCH v2 6/9] platform/chrome: Add event handling Nick Crews
2019-01-14 22:03 ` [PATCH v2 7/9] platform/chrome: Add EC properties Nick Crews
2019-01-14 22:03 ` [PATCH v2 8/9] platform/chrome: Add peakshift and adv_batt_charging Nick Crews
2019-01-14 22:03 ` [PATCH v2 9/9] platform/chrome: Add binary telemetry attributes Nick Crews

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