Re: [PATCH RESEND 2/2] platform/chrome: cros_ec_lpc: Add support for mec1322 EC
Hi Enric,
Thanks for the review.
On 30/11/2016 16:48, Enric Balletbo Serra wrote:
Hi Thierry,
I reviewed your patches and looks good to me, I only found a few style
things that is up to maintainer decide if are needed or not, most of
them are feedback I received on other subsystems. Ah, and I've a
question about runtime detection of the EC (see below), but guess the
answer is no.
Reviewed-by: Enric Balletbo i Serra
2016-11-08 13:27 GMT+01:00 Thierry Escande :
From: Shawn Nematbakhsh
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
Signed-off-by: Gwendal Grignou
Signed-off-by: Guenter Roeck
Signed-off-by: Thierry Escande
---
drivers/platform/chrome/Kconfig | 9 ++
drivers/platform/chrome/Makefile | 1 +
drivers/platform/chrome/cros_ec_lpc.c | 5 ++
drivers/platform/chrome/cros_ec_lpc_mec.c | 144 ++
drivers/platform/chrome/cros_ec_lpc_reg.c | 69 ++
include/linux/mfd/cros_ec_lpc_mec.h | 93 +++
include/linux/mfd/cros_ec_lpc_reg.h | 14 +++
7 files changed, 335 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..55149f2 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -59,6 +59,15 @@ 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.
+
As reported by checkpatch, write a paragraph that describes the config
symbol fully. Maybe adding something like this
If you have a ChromeOS Embedded Controller Microchip EC variant
choose Y here.
According to the help if you have a non-Microchip EC you should leave
this as N. Would be possible some kind of runtime detection of the EC
? Just thinking in out loud.
Well, we can use the EC_CMD_GET_CHIP_INFO command and check for the chip
name as it is "mec1322" (at least on the cyan that I have).
Regards,
Thierry
config CROS_EC_PROTO
bool
help
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 127fbe8..b8f7a3b 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -5,6 +5,7 @@ cros_ec_devs-objs := cros_ec_dev.o
cros_ec_sysfs.o \
cros_ec_lightbar.o cros_ec_vbc.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 617074e..264234b 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -349,10 +349,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;
}
@@ -361,6 +364,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;
}
@@ -371,6 +375,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 000..09e2e21
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_lpc_mec.c
@@ -0,0 +1,144 @@
+/*
+ * cros_ec_lpc_mec - LPC variant I/O for Microchip EC
+ *
+ * Copyright (C) 2015 Google, Inc
+ *
Update the copyright to 2016
+ * This software is licensed under the terms of the GNU General P
Re: [PATCH RESEND 2/2] platform/chrome: cros_ec_lpc: Add support for mec1322 EC
Hi Thierry,
I reviewed your patches and looks good to me, I only found a few style
things that is up to maintainer decide if are needed or not, most of
them are feedback I received on other subsystems. Ah, and I've a
question about runtime detection of the EC (see below), but guess the
answer is no.
Reviewed-by: Enric Balletbo i Serra
2016-11-08 13:27 GMT+01:00 Thierry Escande :
> From: Shawn Nematbakhsh
>
> 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
> Signed-off-by: Gwendal Grignou
> Signed-off-by: Guenter Roeck
> Signed-off-by: Thierry Escande
> ---
> drivers/platform/chrome/Kconfig | 9 ++
> drivers/platform/chrome/Makefile | 1 +
> drivers/platform/chrome/cros_ec_lpc.c | 5 ++
> drivers/platform/chrome/cros_ec_lpc_mec.c | 144
> ++
> drivers/platform/chrome/cros_ec_lpc_reg.c | 69 ++
> include/linux/mfd/cros_ec_lpc_mec.h | 93 +++
> include/linux/mfd/cros_ec_lpc_reg.h | 14 +++
> 7 files changed, 335 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..55149f2 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -59,6 +59,15 @@ 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.
> +
As reported by checkpatch, write a paragraph that describes the config
symbol fully. Maybe adding something like this
If you have a ChromeOS Embedded Controller Microchip EC variant
choose Y here.
According to the help if you have a non-Microchip EC you should leave
this as N. Would be possible some kind of runtime detection of the EC
? Just thinking in out loud.
> config CROS_EC_PROTO
> bool
> help
> diff --git a/drivers/platform/chrome/Makefile
> b/drivers/platform/chrome/Makefile
> index 127fbe8..b8f7a3b 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -5,6 +5,7 @@ cros_ec_devs-objs := cros_ec_dev.o
> cros_ec_sysfs.o \
>cros_ec_lightbar.o cros_ec_vbc.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 617074e..264234b 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -349,10 +349,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;
> }
>
> @@ -361,6 +364,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;
> }
>
> @@ -371,6 +375,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 000..09e2e21
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_lpc_mec.c
> @@ -0,0 +1,144 @@
> +/*
> + * cros_ec_lpc_mec - LPC variant I/O for Microchip EC
> + *
> + * Copyright (C) 2015 Google, Inc
> + *
Update the copyright to 2016
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as publis
[PATCH RESEND 2/2] platform/chrome: cros_ec_lpc: Add support for mec1322 EC
From: Shawn Nematbakhsh
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
Signed-off-by: Gwendal Grignou
Signed-off-by: Guenter Roeck
Signed-off-by: Thierry Escande
---
drivers/platform/chrome/Kconfig | 9 ++
drivers/platform/chrome/Makefile | 1 +
drivers/platform/chrome/cros_ec_lpc.c | 5 ++
drivers/platform/chrome/cros_ec_lpc_mec.c | 144 ++
drivers/platform/chrome/cros_ec_lpc_reg.c | 69 ++
include/linux/mfd/cros_ec_lpc_mec.h | 93 +++
include/linux/mfd/cros_ec_lpc_reg.h | 14 +++
7 files changed, 335 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..55149f2 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -59,6 +59,15 @@ 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.
+
config CROS_EC_PROTO
bool
help
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 127fbe8..b8f7a3b 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -5,6 +5,7 @@ cros_ec_devs-objs := cros_ec_dev.o
cros_ec_sysfs.o \
cros_ec_lightbar.o cros_ec_vbc.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 617074e..264234b 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -349,10 +349,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;
}
@@ -361,6 +364,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;
}
@@ -371,6 +375,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 000..09e2e21
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_lpc_mec.c
@@ -0,0 +1,144 @@
+/*
+ * cros_ec_lpc_mec - LPC variant I/O for Microchip EC
+ *
+ * Copyright (C) 2015 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
+#include
+#include
+#include
+#include
+#include
+
+/*
+ * This mutex must be held while accessing the EMI unit. We can't rely on the
+ * EC mutex becau
