From 22c040fa21b604b9b3d88645e108fb2f0a74474b Mon Sep 17 00:00:00 2001 From: Enric Balletbo i Serra Date: Fri, 14 Jun 2019 23:43:01 +0200 Subject: [PATCH] platform/chrome: cros_ec_lpc: Choose Microchip EC at runtime On many boards, communication between the kernel and the Embedded Controller happens over an LPC bus. In these cases, the kernel config CONFIG_CROS_EC_LPC is enabled. Some of these LPC boards contain a Microchip Embedded Controller (MEC) that is different from the regular EC. On these devices, the same LPC bus is used, but the protocol is a little different. In these cases, the CONFIG_CROS_EC_LPC_MEC kernel config is enabled. Currently, the kernel decides at compile-time whether or not to use the MEC variant, and, when that kernel option is selected it breaks the other boards. We would like a kind of runtime detection to avoid this. This patch adds that detection mechanism by probing the protocol at runtime, first we assume that a MEC variant is connected, and if the protocol fails it fallbacks to the regular EC. This adds a bit of overload because we try to read twice on those LPC boards that doesn't contain a MEC variant, but is a better solution than having to select the EC variant at compile-time. While here also fix the alignment in Kconfig file for this config option replacing the spaces by tabs. Signed-off-by: Enric Balletbo i Serra Reviewed-by: Ezequiel Garcia Tested-by: Nick Crews Reviewed-by: Nick Crews --- drivers/platform/chrome/Kconfig | 29 ++---- drivers/platform/chrome/Makefile | 3 +- drivers/platform/chrome/cros_ec_lpc.c | 158 ++++++++++++++++--------------- drivers/platform/chrome/wilco_ec/Kconfig | 2 +- 4 files changed, 95 insertions(+), 97 deletions(-) diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig index 0b3c2b9..8010e7e 100644 --- a/drivers/platform/chrome/Kconfig +++ b/drivers/platform/chrome/Kconfig @@ -95,28 +95,17 @@ config CROS_EC_SPI 'pre-amble' bytes before the response actually starts. config CROS_EC_LPC - tristate "ChromeOS Embedded Controller (LPC)" - depends on MFD_CROS_EC && ACPI && (X86 || COMPILE_TEST) - help - If you say Y here, you get support for talking to the ChromeOS EC - over an LPC bus. This uses a simple byte-level protocol with a - checksum. This is used for userspace access only. The kernel - typically has its own communication methods. - - 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 + tristate "ChromeOS Embedded Controller (LPC)" + depends on MFD_CROS_EC && ACPI && (X86 || COMPILE_TEST) 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 say Y here, you get support for talking to the ChromeOS EC + over an LPC bus, including the LPC Microchip EC (MEC) variant. + This uses a simple byte-level protocol with a checksum. This is + used for userspace access only. The kernel typically has its own + communication methods. - If you have a ChromeOS Embedded Controller Microchip EC variant - choose Y here. + To compile this driver as a module, choose M here: the + module will be called cros_ec_lpcs. config CROS_EC_PROTO bool diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile index 5a2bcef..fd0af05c 100644 --- a/drivers/platform/chrome/Makefile +++ b/drivers/platform/chrome/Makefile @@ -10,8 +10,7 @@ obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o obj-$(CONFIG_CROS_EC_ISHTP) += cros_ec_ishtp.o obj-$(CONFIG_CROS_EC_RPMSG) += cros_ec_rpmsg.o obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o -cros_ec_lpcs-objs := cros_ec_lpc.o -cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o +cros_ec_lpcs-objs := cros_ec_lpc.o cros_ec_lpc_mec.o obj-$(CONFIG_CROS_EC_LPC) += cros_ec_lpcs.o obj-$(CONFIG_CROS_EC_PROTO) += cros_ec_proto.o cros_ec_trace.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 8d06add..2c44c7f 100644 --- a/drivers/platform/chrome/cros_ec_lpc.c +++ b/drivers/platform/chrome/cros_ec_lpc.c @@ -31,7 +31,26 @@ /* True if ACPI device is present */ static bool cros_ec_lpc_acpi_device_found; -static u8 lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest) +/** + * struct lpc_driver_ops - LPC driver operations + * @read: Copy length bytes from EC address offset into buffer dest. Returns + * the 8-bit checksum of all bytes read. + * @write: Copy length bytes from buffer msg into EC address offset. Returns + * the 8-bit checksum of all bytes written. + */ +struct lpc_driver_ops { + u8 (*read)(unsigned int offset, unsigned int length, u8 *dest); + u8 (*write)(unsigned int offset, unsigned int length, const u8 *msg); +}; + +static struct lpc_driver_ops cros_ec_lpc_ops = { }; + +/* + * A generic instance of the read function of struct lpc_driver_ops, used for + * the LPC EC. + */ +static u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, + u8 *dest) { int sum = 0; int i; @@ -45,7 +64,12 @@ static u8 lpc_read_bytes(unsigned int offset, unsigned int length, u8 *dest) return sum; } -static u8 lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg) +/* + * A generic instance of the write function of struct lpc_driver_ops, used for + * the LPC EC. + */ +static u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, + const u8 *msg) { int sum = 0; int i; @@ -59,10 +83,12 @@ static u8 lpc_write_bytes(unsigned int offset, unsigned int length, u8 *msg) return sum; } -#ifdef CONFIG_CROS_EC_LPC_MEC - -static u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, - u8 *dest) +/* + * An instance of the read function of struct lpc_driver_ops, used for the + * MEC variant of LPC EC. + */ +static u8 cros_ec_lpc_mec_read_bytes(unsigned int offset, unsigned int length, + u8 *dest) { int in_range = cros_ec_lpc_mec_in_range(offset, length); @@ -73,11 +99,15 @@ static u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, cros_ec_lpc_io_bytes_mec(MEC_IO_READ, offset - EC_HOST_CMD_REGION0, length, dest) : - lpc_read_bytes(offset, length, dest); + cros_ec_lpc_read_bytes(offset, length, dest); } -static u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, - u8 *msg) +/* + * An instance of the write function of struct lpc_driver_ops, used for the + * MEC variant of LPC EC. + */ +static u8 cros_ec_lpc_mec_write_bytes(unsigned int offset, unsigned int length, + const u8 *msg) { int in_range = cros_ec_lpc_mec_in_range(offset, length); @@ -87,45 +117,10 @@ static u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, return in_range ? cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, offset - EC_HOST_CMD_REGION0, - length, msg) : - lpc_write_bytes(offset, length, msg); -} - -static void cros_ec_lpc_reg_init(void) -{ - cros_ec_lpc_mec_init(EC_HOST_CMD_REGION0, - EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE); + length, (u8 *)msg) : + cros_ec_lpc_write_bytes(offset, length, msg); } -static void cros_ec_lpc_reg_destroy(void) -{ - cros_ec_lpc_mec_destroy(); -} - -#else /* CONFIG_CROS_EC_LPC_MEC */ - -static u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length, - u8 *dest) -{ - return lpc_read_bytes(offset, length, dest); -} - -static u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length, - u8 *msg) -{ - return lpc_write_bytes(offset, length, msg); -} - -static void cros_ec_lpc_reg_init(void) -{ -} - -static void cros_ec_lpc_reg_destroy(void) -{ -} - -#endif /* CONFIG_CROS_EC_LPC_MEC */ - static int ec_response_timed_out(void) { unsigned long one_second = jiffies + HZ; @@ -133,7 +128,7 @@ static int ec_response_timed_out(void) usleep_range(200, 300); do { - if (!(cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_CMD, 1, &data) & + if (!(cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_CMD, 1, &data) & EC_LPC_STATUS_BUSY_MASK)) return 0; usleep_range(100, 200); @@ -153,11 +148,11 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec, ret = cros_ec_prepare_tx(ec, msg); /* Write buffer */ - cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_PACKET, ret, ec->dout); + cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PACKET, ret, ec->dout); /* Here we go */ sum = EC_COMMAND_PROTOCOL_3; - cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_CMD, 1, &sum); + cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_CMD, 1, &sum); if (ec_response_timed_out()) { dev_warn(ec->dev, "EC responsed timed out\n"); @@ -166,15 +161,15 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec, } /* Check result */ - msg->result = cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_DATA, 1, &sum); + msg->result = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_DATA, 1, &sum); ret = cros_ec_check_result(ec, msg); if (ret) goto done; /* Read back response */ dout = (u8 *)&response; - sum = cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_PACKET, sizeof(response), - dout); + sum = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PACKET, sizeof(response), + dout); msg->result = response.result; @@ -187,9 +182,9 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec, } /* Read response and process checksum */ - sum += cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_PACKET + - sizeof(response), response.data_len, - msg->data); + sum += cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PACKET + + sizeof(response), response.data_len, + msg->data); if (sum) { dev_err(ec->dev, @@ -229,17 +224,17 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec, sum = msg->command + args.flags + args.command_version + args.data_size; /* Copy data and update checksum */ - sum += cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_PARAM, msg->outsize, - msg->data); + sum += cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PARAM, msg->outsize, + msg->data); /* Finalize checksum and write args */ args.checksum = sum; - cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_ARGS, sizeof(args), - (u8 *)&args); + cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_ARGS, sizeof(args), + (u8 *)&args); /* Here we go */ sum = msg->command; - cros_ec_lpc_write_bytes(EC_LPC_ADDR_HOST_CMD, 1, &sum); + cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_CMD, 1, &sum); if (ec_response_timed_out()) { dev_warn(ec->dev, "EC responsed timed out\n"); @@ -248,14 +243,13 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec, } /* Check result */ - msg->result = cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_DATA, 1, &sum); + msg->result = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_DATA, 1, &sum); ret = cros_ec_check_result(ec, msg); if (ret) goto done; /* Read back args */ - cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_ARGS, sizeof(args), - (u8 *)&args); + cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_ARGS, sizeof(args), (u8 *)&args); if (args.data_size > msg->insize) { dev_err(ec->dev, @@ -269,8 +263,8 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec, sum = msg->command + args.flags + args.command_version + args.data_size; /* Read response and update checksum */ - sum += cros_ec_lpc_read_bytes(EC_LPC_ADDR_HOST_PARAM, args.data_size, - msg->data); + sum += cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PARAM, args.data_size, + msg->data); /* Verify checksum */ if (args.checksum != sum) { @@ -300,13 +294,13 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset, /* fixed length */ if (bytes) { - cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + offset, bytes, s); + cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + offset, bytes, s); return bytes; } /* string */ for (; i < EC_MEMMAP_SIZE; i++, s++) { - cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + i, 1, s); + cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + i, 1, s); cnt++; if (!*s) break; @@ -343,10 +337,25 @@ static int cros_ec_lpc_probe(struct platform_device *pdev) return -EBUSY; } - cros_ec_lpc_read_bytes(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf); + /* + * Read the mapped ID twice, the first one is assuming the + * EC is a Microchip Embedded Controller (MEC) variant, if the + * protocol fails, fallback to the non MEC variant and try to + * read again the ID. + */ + cros_ec_lpc_ops.read = cros_ec_lpc_mec_read_bytes; + cros_ec_lpc_ops.write = cros_ec_lpc_mec_write_bytes; + cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf); if (buf[0] != 'E' || buf[1] != 'C') { - dev_err(dev, "EC ID not detected\n"); - return -ENODEV; + /* Re-assign read/write operations for the non MEC variant */ + cros_ec_lpc_ops.read = cros_ec_lpc_read_bytes; + cros_ec_lpc_ops.write = cros_ec_lpc_write_bytes; + cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, + buf); + if (buf[0] != 'E' || buf[1] != 'C') { + dev_err(dev, "EC ID not detected\n"); + return -ENODEV; + } } if (!devm_request_region(dev, EC_HOST_CMD_REGION0, @@ -541,13 +550,14 @@ static int __init cros_ec_lpc_init(void) return -ENODEV; } - cros_ec_lpc_reg_init(); + cros_ec_lpc_mec_init(EC_HOST_CMD_REGION0, + EC_LPC_ADDR_MEMMAP + EC_MEMMAP_SIZE); /* 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(); + cros_ec_lpc_mec_destroy(); return ret; } @@ -557,7 +567,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(); + cros_ec_lpc_mec_destroy(); } } @@ -569,7 +579,7 @@ static void __exit cros_ec_lpc_exit(void) if (!cros_ec_lpc_acpi_device_found) platform_device_unregister(&cros_ec_lpc_device); platform_driver_unregister(&cros_ec_lpc_driver); - cros_ec_lpc_reg_destroy(); + cros_ec_lpc_mec_destroy(); } module_init(cros_ec_lpc_init); diff --git a/drivers/platform/chrome/wilco_ec/Kconfig b/drivers/platform/chrome/wilco_ec/Kconfig index 9033687..5b7e180 100644 --- a/drivers/platform/chrome/wilco_ec/Kconfig +++ b/drivers/platform/chrome/wilco_ec/Kconfig @@ -1,6 +1,6 @@ config WILCO_EC tristate "ChromeOS Wilco Embedded Controller" - depends on ACPI && X86 && CROS_EC_LPC && CROS_EC_LPC_MEC + depends on ACPI && X86 && CROS_EC_LPC 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 -- 2.7.4