Date:   Mon, 18 Feb 2019 11:13:05 +0100
From:   Enric Balletbo Serra <eballetbo@...il.com>
To:     Nick Crews <ncrews@...omium.org>
Cc:     linux-kernel <linux-kernel@...r.kernel.org>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Simon Glass <sjg@...omium.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Guenter Roeck <groeck@...omium.org>, dlaurie@...omium.org,
        linux-rtc@...r.kernel.org,
        Enric Balletbo i Serra <enric.balletbo@...labora.com>,
        Alessandro Zummo <a.zummo@...ertech.it>,
        Benson Leung <bleung@...omium.org>,
        Duncan Laurie <dlaurie@...gle.com>
Subject: Re: [PATCH v8 0/4] platform/chrome: Add basic support for Wilco EC

Missatge de Nick Crews <ncrews@...omium.org> del dia ds., 9 de febr.
2019 a les 1:37:
>
>
> There is a new chromebook that contains a different Embedded Controller
> (codename Wilco) than the rest of the chromebook series. Thus the kernel
> requires a different driver than the already existing and generalized
> cros_ec_* drivers. The core of the communication with the EC
> is implemented in wilco_ec/mailbox.c, using a simple byte-level protocol
> with a checksum, transmitted over an eSPI bus.
>
> In this patch series I only introduce a very barebones
> driver with a debugfs interface for sending/receiving raw byte
> sequences to the EC, as well as a standard RTC driver with read/write
> functionality.  Later, I will hopefully include more specialized
> drivers such as a keyboard backlight driver, an EC event notification node,
> telemetry data query node, etc.
>
> The entry point of the driver is wilco_ec/core.c,
> which is responsible for several tasks:
> - Initialize the register interface that is used by wilco_ec_mailbox()
> - Create a platform device which is picked up by the debugfs driver
> - Create a platform device which is picked up by the RTC driver
>
> A full thread of the development of these patches can be found at
> https://chromium-review.googlesource.com/c/1356082. This thread contains
> comments and revisions that could be helpful in understanding how the
> driver arrived at the state it is in now. The thread also contains some
> ChromeOS specific patches that actually enable the driver. If you want
> to test the patch yourself, you would have to install the ChromeOS SDK
> and cherry pick in these patches, and even then you would need a Sarien
> device to actually run it.
>
> Thank you for your comments!
>
> Changes in v8:
> - Fix a comment about registering the debugfs driver
>
> Changes in v7:
> - Switch to #define for MAX_WORD_SIZE so array size
>   can be determined at compile time.
>
> Changes in v6:
> - Re-added WILCO_EC_FLAG_EXTENDED_DATA and went back to always
>   reading either EC_MAILBOX_DATA_SIZE or EC_MAILBOX_DATA_SIZE_EXTENDED
>   bytes, since it turns out the EC always expects you to calculate the
>   checksum over all of them.
> - Split WILCO_EC_MSG_TELEMETRY into WILCO_EC_MSG_TELEMETRY_SHORT and
>   WILCO_EC_MSG_TELEMETRY_LONG, since there will be two different
>   commands.
> - A couple comment and err message polishes
> - s/4.19/5.1/ for kernel version in documentation, since that is
>   the version this patch should land in.
> - Instead of requiring at least 3 bytes for msg type and command,
>   now just require two for msg type. We can skip the command.
> - Fixed error checking in probe() so that errors are hidden, without
>   causing more errors or unextpected behavior.
> - Some comment polishing.
> - In the core, actually unregister the debugfs child platform_device
> - In the core, actually unregister the RTC child platform_device.
>
> Changes in v5:
> - move checking of NO_RESPONSE flag before timeout check,
>   so now timeout doesn't always happen when EC isn't supposed
>   to respond.
> - rm WILCO_EC_FLAG_EXTENDED_DATA, that is already obvious from
>   wilco_ec_message.response_size
> - core now always continues regardless of debugfs failure
> - mv documentation to file header
> - Check for OOM
> - rm unneeded check if debug_info is allocqated
> - rm bogus comment
> - add space around "+"
> - rm WILCO_EC_FLAG_EXTENDED_DATA, that is already obvious from
>   wilco_ec_message.response_size
>
> Changes in v4:
> - add #define DRV_NAME
> - remove redundant Kconfig nesting
> - changed my email to @chromium.org
> - Add better explanation of what core.c does
> - Change debugfs driver to be a separate module
> - Change me email to @chromium.org from @google.com
> - Change CONFIG_WILCO_EC_SYSFS_RAW to
>   CONFIG_WILCO_EC_DEBUGFS
> - Change me email to @chromium.org from @google.com
> - Move "Add RTC driver" before "Add sysfs attributes" so that
>   it could get accepted earlier, since it is less contentious
>
> Changes in v3:
> - Change <= to >= in mec_in_range()
> - Add " - EC_HOST_CMD_REGION0" to offset arg for io_bytes_mec()
> - remove unused ret in probe()
> - Add newline spacer in probe()
> - rm unnecessary res in get_resource()
> - s/8bit/8-bit
> - rm first sleep when sending command to EC
> - Move the attribute to the debugfs system
> - Move the implementation to debugfs.c
> - Improve the raw hex parsing
> - Encapsulate global variables in one object
> - Add safety check when passing less than 3 bytes
> - Move documentation to debugfs-wilco-ec
> - rm #define for driver name
> - Don't compute weekday when reading from RTC.
>   Still set weekday when writing, as RTC needs this
>   to control advanced power scheduling
> - rm check for invalid month data
> - Set range_min and range_max on rtc_device
>
> Changes in v2:
> - Fixed kernel-doc comments
> - Fixed include of linux/mfd/cros_ec_lpc_mec.h
> - cros_ec_lpc_mec_in_range() returns -EINVAL on error
> - Added parens around macro variables
> - Remove COMPILE_TEST from Kconfig because inb()/outb()
> won't work on anything but X86
> - Add myself as module author
> - Tweak mailbox()
> - Add sysfs documentation
> - rm duplicate EC_MAILBOX_DATA_SIZE defs
> - Make docstrings follow kernel style
> - Fix tags in commit msg
> - Move Kconfig to subdirectory
> - Reading raw now includes ASCII translation
> - rm license boiler plate
> - rm "wilco_ec_rtc -" prefix in docstring
> - Make rtc driver its own module within the drivers/rtc/ directory
> - Register a rtc device from core.c that is picked up by this driver
>
> Nick Crews (4):
>   cros_ec: Remove cros_ec dependency in lpc_mec
>   platform/chrome: Add new driver for Wilco EC
>   platform/chrome: Add support for raw commands in debugfs
>   platform/chrome: rtc: Add RTC driver
>
>  Documentation/ABI/testing/debugfs-wilco-ec |  23 ++
>  drivers/platform/chrome/Kconfig            |   4 +-
>  drivers/platform/chrome/Makefile           |   2 +
>  drivers/platform/chrome/cros_ec_lpc_mec.c  |  52 ++++-
>  drivers/platform/chrome/cros_ec_lpc_mec.h  |  43 ++--
>  drivers/platform/chrome/cros_ec_lpc_reg.c  |  47 ++--
>  drivers/platform/chrome/wilco_ec/Kconfig   |  21 ++
>  drivers/platform/chrome/wilco_ec/Makefile  |   6 +
>  drivers/platform/chrome/wilco_ec/core.c    | 136 ++++++++++++
>  drivers/platform/chrome/wilco_ec/debugfs.c | 238 +++++++++++++++++++++
>  drivers/platform/chrome/wilco_ec/mailbox.c | 236 ++++++++++++++++++++
>  drivers/rtc/Kconfig                        |  11 +
>  drivers/rtc/Makefile                       |   1 +
>  drivers/rtc/rtc-wilco-ec.c                 | 177 +++++++++++++++
>  include/linux/platform_data/wilco-ec.h     | 144 +++++++++++++
>  15 files changed, 1081 insertions(+), 60 deletions(-)
>  create mode 100644 Documentation/ABI/testing/debugfs-wilco-ec
>  create mode 100644 drivers/platform/chrome/wilco_ec/Kconfig
>  create mode 100644 drivers/platform/chrome/wilco_ec/Makefile
>  create mode 100644 drivers/platform/chrome/wilco_ec/core.c
>  create mode 100644 drivers/platform/chrome/wilco_ec/debugfs.c
>  create mode 100644 drivers/platform/chrome/wilco_ec/mailbox.c
>  create mode 100644 drivers/rtc/rtc-wilco-ec.c
>  create mode 100644 include/linux/platform_data/wilco-ec.h
>
> --
> 2.20.1.791.gb4d0f1c61a-goog
>

Applied for 5.1

Thanks

Powered by blists - more mailing lists