Message-ID: <20170622190144.GA78452@decatoncale.mtv.corp.google.com>
Date:   Thu, 22 Jun 2017 12:01:44 -0700
From:   Benson Leung <bleung@...gle.com>
To:     Enric Balletbo i Serra <enric.balletbo@...labora.com>
Cc:     olof@...om.net, bleung@...omium.org, linux-kernel@...r.kernel.org,
        lee.jones@...aro.org, Shawn Nematbakhsh <shawnn@...omium.org>,
        Thierry Escande <thierry.escande@...labora.com>
Subject: Re: [PATCH RESEND 06/13] platform/chrome: cros_ec_lpc: Add support
 for mec1322 EC

Hi Enric,

This patch plus the mec patch lgtm. Thanks!

On Tue, May 16, 2017 at 06:13:12PM +0200, Enric Balletbo i Serra wrote:
> From: Shawn Nematbakhsh <shawnn@...omium.org>
> 
> This adds support for the ChromeOS LPC Microchip Embedded Controller
> (mec1322) variant.
> 
> mec1322 accesses I/O region [800h, 9ffh] through embedded memory
> interface (EMI) rather than LPC.
> 
> Signed-off-by: Shawn Nematbakhsh <shawnn@...omium.org>
> Signed-off-by: Thierry Escande <thierry.escande@...labora.com>

Signed-off-by: Benson Leung <bleung@...omium.org>

Applied to my upcoming immutable branch.

> ---
>  drivers/platform/chrome/Kconfig           |  12 +++
>  drivers/platform/chrome/Makefile          |   1 +
>  drivers/platform/chrome/cros_ec_lpc.c     |   5 ++
>  drivers/platform/chrome/cros_ec_lpc_mec.c | 140 ++++++++++++++++++++++++++++++
>  drivers/platform/chrome/cros_ec_lpc_reg.c |  69 +++++++++++++++
>  include/linux/mfd/cros_ec_lpc_mec.h       |  90 +++++++++++++++++++
>  include/linux/mfd/cros_ec_lpc_reg.h       |  14 +++
>  7 files changed, 331 insertions(+)
>  create mode 100644 drivers/platform/chrome/cros_ec_lpc_mec.c
>  create mode 100644 include/linux/mfd/cros_ec_lpc_mec.h
> 
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 76bdae1..6d80fb5 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -59,6 +59,18 @@ config CROS_EC_LPC
>            To compile this driver as a module, choose M here: the
>            module will be called cros_ec_lpc.
>  
> +config CROS_EC_LPC_MEC
> +	bool "ChromeOS Embedded Controller LPC Microchip EC (MEC) variant"
> +	depends on CROS_EC_LPC
> +	default n
> +	help
> +	  If you say Y here, a variant LPC protocol for the Microchip EC
> +	  will be used. Note that this variant is not backward compatible
> +	  with non-Microchip ECs.
> +
> +	  If you have a ChromeOS Embedded Controller Microchip EC variant
> +	  choose Y here.
> +
>  config CROS_EC_PROTO
>          bool
>          help
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index 61182fd..66c345c 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -6,6 +6,7 @@ cros_ec_devs-objs			:= cros_ec_dev.o cros_ec_sysfs.o \
>  					   cros_ec_debugfs.o
>  obj-$(CONFIG_CROS_EC_CHARDEV)		+= cros_ec_devs.o
>  cros_ec_lpcs-objs			:= cros_ec_lpc.o cros_ec_lpc_reg.o
> +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
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 6a782a6..bc2dc62 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -346,10 +346,13 @@ static int __init cros_ec_lpc_init(void)
>  		return -ENODEV;
>  	}
>  
> +	cros_ec_lpc_reg_init();
> +
>  	/* Register the driver */
>  	ret = platform_driver_register(&cros_ec_lpc_driver);
>  	if (ret) {
>  		pr_err(DRV_NAME ": can't register driver: %d\n", ret);
> +		cros_ec_lpc_reg_destroy();
>  		return ret;
>  	}
>  
> @@ -358,6 +361,7 @@ static int __init cros_ec_lpc_init(void)
>  	if (ret) {
>  		pr_err(DRV_NAME ": can't register device: %d\n", ret);
>  		platform_driver_unregister(&cros_ec_lpc_driver);
> +		cros_ec_lpc_reg_destroy();
>  		return ret;
>  	}
>  
> @@ -368,6 +372,7 @@ static void __exit cros_ec_lpc_exit(void)
>  {
>  	platform_device_unregister(&cros_ec_lpc_device);
>  	platform_driver_unregister(&cros_ec_lpc_driver);
> +	cros_ec_lpc_reg_destroy();
>  }
>  
>  module_init(cros_ec_lpc_init);
> diff --git a/drivers/platform/chrome/cros_ec_lpc_mec.c b/drivers/platform/chrome/cros_ec_lpc_mec.c
> new file mode 100644
> index 0000000..2eda2c2
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_lpc_mec.c
> @@ -0,0 +1,140 @@
> +/*
> + * cros_ec_lpc_mec - LPC variant I/O for Microchip EC
> + *
> + * Copyright (C) 2016 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the Chrome OS EC byte-level message-based protocol for
> + * communicating the keyboard state (which keys are pressed) from a keyboard EC
> + * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
> + * but everything else (including deghosting) is done here.  The main
> + * motivation for this is to keep the EC firmware as simple as possible, since
> + * it cannot be easily upgraded and EC flash/IRAM space is relatively
> + * expensive.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/mfd/cros_ec_lpc_mec.h>
> +#include <linux/mutex.h>
> +#include <linux/types.h>
> +
> +/*
> + * This mutex must be held while accessing the EMI unit. We can't rely on the
> + * EC mutex because memmap data may be accessed without it being held.
> + */
> +static struct mutex io_mutex;
> +
> +/*
> + * cros_ec_lpc_mec_emi_write_address
> + *
> + * Initialize EMI read / write at a given address.
> + *
> + * @addr:        Starting read / write address
> + * @access_type: Type of access, typically 32-bit auto-increment
> + */
> +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);
> +}
> +
> +/*
> + * cros_ec_lpc_io_bytes_mec - Read / write bytes to MEC EMI port
> + *
> + * @io_type: MEC_IO_READ or MEC_IO_WRITE, depending on request
> + * @offset:  Base read / write address
> + * @length:  Number of bytes to read / write
> + * @buf:     Destination / source buffer
> + *
> + * @return 8-bit checksum of all bytes read / written
> + */
> +u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
> +			    unsigned int offset, unsigned int length,
> +			    u8 *buf)
> +{
> +	int i = 0;
> +	int io_addr;
> +	u8 sum = 0;
> +	enum cros_ec_lpc_mec_emi_access_mode access, new_access;
> +
> +	/*
> +	 * Long access cannot be used on misaligned data since reading B0 loads
> +	 * the data register and writing B3 flushes.
> +	 */
> +	if (offset & 0x3 || length < 4)
> +		access = ACCESS_TYPE_BYTE;
> +	else
> +		access = ACCESS_TYPE_LONG_AUTO_INCREMENT;
> +
> +	mutex_lock(&io_mutex);
> +
> +	/* Initialize I/O at desired address */
> +	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);
> +	while (i < length) {
> +		while (io_addr <= MEC_EMI_EC_DATA_B3) {
> +			if (io_type == MEC_IO_READ)
> +				buf[i] = inb(io_addr++);
> +			else
> +				outb(buf[i], io_addr++);
> +
> +			sum += buf[i++];
> +			offset++;
> +
> +			/* Extra bounds check in case of misaligned length */
> +			if (i == length)
> +				goto done;
> +		}
> +
> +		/*
> +		 * Use long auto-increment access except for misaligned write,
> +		 * since writing B3 triggers the flush.
> +		 */
> +		if (length - i < 4 && io_type == MEC_IO_WRITE)
> +			new_access = ACCESS_TYPE_BYTE;
> +		else
> +			new_access = ACCESS_TYPE_LONG_AUTO_INCREMENT;
> +
> +		if (new_access != access ||
> +		    access != ACCESS_TYPE_LONG_AUTO_INCREMENT) {
> +			access = new_access;
> +			cros_ec_lpc_mec_emi_write_address(offset, access);
> +		}
> +
> +		/* Access [B0, B3] on each loop pass */
> +		io_addr = MEC_EMI_EC_DATA_B0;
> +	}
> +
> +done:
> +	mutex_unlock(&io_mutex);
> +
> +	return sum;
> +}
> +EXPORT_SYMBOL(cros_ec_lpc_io_bytes_mec);
> +
> +void cros_ec_lpc_mec_init(void)
> +{
> +	mutex_init(&io_mutex);
> +}
> +EXPORT_SYMBOL(cros_ec_lpc_mec_init);
> +
> +void cros_ec_lpc_mec_destroy(void)
> +{
> +	mutex_destroy(&io_mutex);
> +}
> +EXPORT_SYMBOL(cros_ec_lpc_mec_destroy);
> diff --git a/drivers/platform/chrome/cros_ec_lpc_reg.c b/drivers/platform/chrome/cros_ec_lpc_reg.c
> index 03c9781..dcc7a3e 100644
> --- a/drivers/platform/chrome/cros_ec_lpc_reg.c
> +++ b/drivers/platform/chrome/cros_ec_lpc_reg.c
> @@ -24,6 +24,7 @@
>  #include <linux/io.h>
>  #include <linux/mfd/cros_ec.h>
>  #include <linux/mfd/cros_ec_commands.h>
> +#include <linux/mfd/cros_ec_lpc_mec.h>
>  
>  static u8 lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
>  {
> @@ -53,12 +54,80 @@ static u8 lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
>  	return sum;
>  }
>  
> +#ifdef CONFIG_CROS_EC_LPC_MEC
> +
>  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;
> +
> +		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))
> +		return 0;
> +
>  	return 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;
> +
> +		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))
> +		return 0;
> +
>  	return lpc_write_bytes(offset, length, msg);
>  }
> +
> +void cros_ec_lpc_reg_init(void)
> +{
> +	cros_ec_lpc_mec_init();
> +}
> +
> +void cros_ec_lpc_reg_destroy(void)
> +{
> +	cros_ec_lpc_mec_destroy();
> +}
> +
> +#else /* CONFIG_CROS_EC_LPC_MEC */
> +
> +u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest)
> +{
> +	return lpc_read_bytes(offset, length, dest);
> +}
> +
> +u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg)
> +{
> +	return lpc_write_bytes(offset, length, msg);
> +}
> +
> +void cros_ec_lpc_reg_init(void)
> +{
> +}
> +
> +void cros_ec_lpc_reg_destroy(void)
> +{
> +}
> +
> +#endif /* CONFIG_CROS_EC_LPC_MEC */
> diff --git a/include/linux/mfd/cros_ec_lpc_mec.h b/include/linux/mfd/cros_ec_lpc_mec.h
> new file mode 100644
> index 0000000..176496d
> --- /dev/null
> +++ b/include/linux/mfd/cros_ec_lpc_mec.h
> @@ -0,0 +1,90 @@
> +/*
> + * cros_ec_lpc_mec - LPC variant I/O for Microchip EC
> + *
> + * Copyright (C) 2016 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the Chrome OS EC byte-level message-based protocol for
> + * communicating the keyboard state (which keys are pressed) from a keyboard EC
> + * to the AP over some bus (such as i2c, lpc, spi).  The EC does debouncing,
> + * but everything else (including deghosting) is done here.  The main
> + * motivation for this is to keep the EC firmware as simple as possible, since
> + * it cannot be easily upgraded and EC flash/IRAM space is relatively
> + * expensive.
> + */
> +
> +#ifndef __LINUX_MFD_CROS_EC_MEC_H
> +#define __LINUX_MFD_CROS_EC_MEC_H
> +
> +#include <linux/mfd/cros_ec_commands.h>
> +
> +enum cros_ec_lpc_mec_emi_access_mode {
> +	/* 8-bit access */
> +	ACCESS_TYPE_BYTE = 0x0,
> +	/* 16-bit access */
> +	ACCESS_TYPE_WORD = 0x1,
> +	/* 32-bit access */
> +	ACCESS_TYPE_LONG = 0x2,
> +	/*
> +	 * 32-bit access, read or write of MEC_EMI_EC_DATA_B3 causes the
> +	 * EC data register to be incremented.
> +	 */
> +	ACCESS_TYPE_LONG_AUTO_INCREMENT = 0x3,
> +};
> +
> +enum cros_ec_lpc_mec_io_type {
> +	MEC_IO_READ,
> +	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)
> +
> +/*
> + * cros_ec_lpc_mec_init
> + *
> + * Initialize MEC I/O.
> + */
> +void cros_ec_lpc_mec_init(void);
> +
> +/*
> + * cros_ec_lpc_mec_destroy
> + *
> + * Cleanup MEC I/O.
> + */
> +void cros_ec_lpc_mec_destroy(void);
> +
> +/**
> + * cros_ec_lpc_io_bytes_mec - Read / write bytes to MEC EMI port
> + *
> + * @io_type: MEC_IO_READ or MEC_IO_WRITE, depending on request
> + * @offset:  Base read / write address
> + * @length:  Number of bytes to read / write
> + * @buf:     Destination / source buffer
> + *
> + * @return 8-bit checksum of all bytes read / written
> + */
> +u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
> +			    unsigned int offset, unsigned int length, u8 *buf);
> +
> +#endif /* __LINUX_MFD_CROS_EC_MEC_H */
> diff --git a/include/linux/mfd/cros_ec_lpc_reg.h b/include/linux/mfd/cros_ec_lpc_reg.h
> index 4089bd5..5560bef 100644
> --- a/include/linux/mfd/cros_ec_lpc_reg.h
> +++ b/include/linux/mfd/cros_ec_lpc_reg.h
> @@ -44,4 +44,18 @@ u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest);
>   */
>  u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg);
>  
> +/**
> + * cros_ec_lpc_reg_init
> + *
> + * Initialize register I/O.
> + */
> +void cros_ec_lpc_reg_init(void);
> +
> +/**
> + * cros_ec_lpc_reg_destroy
> + *
> + * Cleanup reg I/O.
> + */
> +void cros_ec_lpc_reg_destroy(void);
> +
>  #endif /* __LINUX_MFD_CROS_EC_REG_H */
> -- 
> 2.9.3
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@...gle.com
Chromium OS Project
bleung@...omium.org

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)

Powered by blists - more mailing lists