* [PATCH v2 1/6] mfd: Add ChromeOS EC messages header
2013-02-13 2:42 [PATCH v2 0/6] Add ChromeOS Embedded Controller support Simon Glass
@ 2013-02-13 2:42 ` Simon Glass
2013-02-13 2:42 ` [PATCH v2 2/6] mfd: Add ChromeOS EC implementation Simon Glass
` (4 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2013-02-13 2:42 UTC (permalink / raw)
To: LKML; +Cc: Samuel Ortiz, Simon Glass, Che-Liang Chiou, Vincent Palatin
This file is included verbatim from the ChromeOS EC respository.
Ideally we would prefer to avoid changing it, to make it easier
to track this rapidly-changing file.
Signed-off-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Che-Liang Chiou <clchiou@chromium.org>
Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
---
Changes in v2: None
include/linux/mfd/cros_ec_commands.h | 1369 ++++++++++++++++++++++++++++++++++
1 file changed, 1369 insertions(+)
create mode 100644 include/linux/mfd/cros_ec_commands.h
diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
new file mode 100644
index 0000000..86fd069
--- /dev/null
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -0,0 +1,1369 @@
+/*
+ * Host communication command constants for ChromeOS EC
+ *
+ * Copyright (C) 2012 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.
+ *
+ * The ChromeOS EC multi function device is used to mux all the requests
+ * to the EC device for its multiple features: keyboard controller,
+ * battery charging and regulator control, firmware update.
+ *
+ * NOTE: This file is copied verbatim from the ChromeOS EC Open Source
+ * project in an attempt to make future updates easy to make.
+ */
+
+#ifndef __CROS_EC_COMMANDS_H
+#define __CROS_EC_COMMANDS_H
+
+/*
+ * Protocol overview
+ *
+ * request: CMD [ P0 P1 P2 ... Pn S ]
+ * response: ERR [ P0 P1 P2 ... Pn S ]
+ *
+ * where the bytes are defined as follow :
+ * - CMD is the command code. (defined by EC_CMD_ constants)
+ * - ERR is the error code. (defined by EC_RES_ constants)
+ * - Px is the optional payload.
+ * it is not sent if the error code is not success.
+ * (defined by ec_params_ and ec_response_ structures)
+ * - S is the checksum which is the sum of all payload bytes.
+ *
+ * On LPC, CMD and ERR are sent/received at EC_LPC_ADDR_KERNEL|USER_CMD
+ * and the payloads are sent/received at EC_LPC_ADDR_KERNEL|USER_PARAM.
+ * On I2C, all bytes are sent serially in the same message.
+ */
+
+/* Current version of this protocol */
+#define EC_PROTO_VERSION 0x00000002
+
+/* Command version mask */
+#define EC_VER_MASK(version) (1UL << (version))
+
+/* I/O addresses for ACPI commands */
+#define EC_LPC_ADDR_ACPI_DATA 0x62
+#define EC_LPC_ADDR_ACPI_CMD 0x66
+
+/* I/O addresses for host command */
+#define EC_LPC_ADDR_HOST_DATA 0x200
+#define EC_LPC_ADDR_HOST_CMD 0x204
+
+/* I/O addresses for host command args and params */
+#define EC_LPC_ADDR_HOST_ARGS 0x800
+#define EC_LPC_ADDR_HOST_PARAM 0x804
+#define EC_HOST_PARAM_SIZE 0x0fc /* Size of param area in bytes */
+
+/* I/O addresses for host command params, old interface */
+#define EC_LPC_ADDR_OLD_PARAM 0x880
+#define EC_OLD_PARAM_SIZE 0x080 /* Size of param area in bytes */
+
+/* EC command register bit functions */
+#define EC_LPC_CMDR_DATA (1 << 0) /* Data ready for host to read */
+#define EC_LPC_CMDR_PENDING (1 << 1) /* Write pending to EC */
+#define EC_LPC_CMDR_BUSY (1 << 2) /* EC is busy processing a command */
+#define EC_LPC_CMDR_CMD (1 << 3) /* Last host write was a command */
+#define EC_LPC_CMDR_ACPI_BRST (1 << 4) /* Burst mode (not used) */
+#define EC_LPC_CMDR_SCI (1 << 5) /* SCI event is pending */
+#define EC_LPC_CMDR_SMI (1 << 6) /* SMI event is pending */
+
+#define EC_LPC_ADDR_MEMMAP 0x900
+#define EC_MEMMAP_SIZE 255 /* ACPI IO buffer max is 255 bytes */
+#define EC_MEMMAP_TEXT_MAX 8 /* Size of a string in the memory map */
+
+/* The offset address of each type of data in mapped memory. */
+#define EC_MEMMAP_TEMP_SENSOR 0x00 /* Temp sensors */
+#define EC_MEMMAP_FAN 0x10 /* Fan speeds */
+#define EC_MEMMAP_TEMP_SENSOR_B 0x18 /* Temp sensors (second set) */
+#define EC_MEMMAP_ID 0x20 /* 'E' 'C' */
+#define EC_MEMMAP_ID_VERSION 0x22 /* Version of data in 0x20 - 0x2f */
+#define EC_MEMMAP_THERMAL_VERSION 0x23 /* Version of data in 0x00 - 0x1f */
+#define EC_MEMMAP_BATTERY_VERSION 0x24 /* Version of data in 0x40 - 0x7f */
+#define EC_MEMMAP_SWITCHES_VERSION 0x25 /* Version of data in 0x30 - 0x33 */
+#define EC_MEMMAP_EVENTS_VERSION 0x26 /* Version of data in 0x34 - 0x3f */
+#define EC_MEMMAP_HOST_CMD_FLAGS 0x27 /* Host command interface flags */
+#define EC_MEMMAP_SWITCHES 0x30
+#define EC_MEMMAP_HOST_EVENTS 0x34
+#define EC_MEMMAP_BATT_VOLT 0x40 /* Battery Present Voltage */
+#define EC_MEMMAP_BATT_RATE 0x44 /* Battery Present Rate */
+#define EC_MEMMAP_BATT_CAP 0x48 /* Battery Remaining Capacity */
+#define EC_MEMMAP_BATT_FLAG 0x4c /* Battery State, defined below */
+#define EC_MEMMAP_BATT_DCAP 0x50 /* Battery Design Capacity */
+#define EC_MEMMAP_BATT_DVLT 0x54 /* Battery Design Voltage */
+#define EC_MEMMAP_BATT_LFCC 0x58 /* Battery Last Full Charge Capacity */
+#define EC_MEMMAP_BATT_CCNT 0x5c /* Battery Cycle Count */
+#define EC_MEMMAP_BATT_MFGR 0x60 /* Battery Manufacturer String */
+#define EC_MEMMAP_BATT_MODEL 0x68 /* Battery Model Number String */
+#define EC_MEMMAP_BATT_SERIAL 0x70 /* Battery Serial Number String */
+#define EC_MEMMAP_BATT_TYPE 0x78 /* Battery Type String */
+
+/* Number of temp sensors at EC_MEMMAP_TEMP_SENSOR */
+#define EC_TEMP_SENSOR_ENTRIES 16
+/*
+ * Number of temp sensors at EC_MEMMAP_TEMP_SENSOR_B.
+ *
+ * Valid only if EC_MEMMAP_THERMAL_VERSION returns >= 2.
+ */
+#define EC_TEMP_SENSOR_B_ENTRIES 8
+#define EC_TEMP_SENSOR_NOT_PRESENT 0xff
+#define EC_TEMP_SENSOR_ERROR 0xfe
+#define EC_TEMP_SENSOR_NOT_POWERED 0xfd
+#define EC_TEMP_SENSOR_NOT_CALIBRATED 0xfc
+/*
+ * The offset of temperature value stored in mapped memory. This allows
+ * reporting a temperature range of 200K to 454K = -73C to 181C.
+ */
+#define EC_TEMP_SENSOR_OFFSET 200
+
+#define EC_FAN_SPEED_ENTRIES 4 /* Number of fans at EC_MEMMAP_FAN */
+#define EC_FAN_SPEED_NOT_PRESENT 0xffff /* Entry not present */
+#define EC_FAN_SPEED_STALLED 0xfffe /* Fan stalled */
+
+/* Battery bit flags at EC_MEMMAP_BATT_FLAG. */
+#define EC_BATT_FLAG_AC_PRESENT 0x01
+#define EC_BATT_FLAG_BATT_PRESENT 0x02
+#define EC_BATT_FLAG_DISCHARGING 0x04
+#define EC_BATT_FLAG_CHARGING 0x08
+#define EC_BATT_FLAG_LEVEL_CRITICAL 0x10
+
+/* Switch flags at EC_MEMMAP_SWITCHES */
+#define EC_SWITCH_LID_OPEN 0x01
+#define EC_SWITCH_POWER_BUTTON_PRESSED 0x02
+#define EC_SWITCH_WRITE_PROTECT_DISABLED 0x04
+/* Recovery requested via keyboard */
+#define EC_SWITCH_KEYBOARD_RECOVERY 0x08
+/* Recovery requested via dedicated signal (from servo board) */
+#define EC_SWITCH_DEDICATED_RECOVERY 0x10
+/* Was fake developer mode switch; now unused. Remove in next refactor. */
+#define EC_SWITCH_IGNORE0 0x20
+
+/* Host command interface flags */
+/* Host command interface supports LPC args (LPC interface only) */
+#define EC_HOST_CMD_FLAG_LPC_ARGS_SUPPORTED 0x01
+
+/* Wireless switch flags */
+#define EC_WIRELESS_SWITCH_WLAN 0x01
+#define EC_WIRELESS_SWITCH_BLUETOOTH 0x02
+
+/*
+ * This header file is used in coreboot both in C and ACPI code. The ACPI code
+ * is pre-processed to handle constants but the ASL compiler is unable to
+ * handle actual C code so keep it separate.
+ */
+#ifndef __ACPI__
+
+/* LPC command status byte masks */
+/* EC has written a byte in the data register and host hasn't read it yet */
+#define EC_LPC_STATUS_TO_HOST 0x01
+/* Host has written a command/data byte and the EC hasn't read it yet */
+#define EC_LPC_STATUS_FROM_HOST 0x02
+/* EC is processing a command */
+#define EC_LPC_STATUS_PROCESSING 0x04
+/* Last write to EC was a command, not data */
+#define EC_LPC_STATUS_LAST_CMD 0x08
+/* EC is in burst mode. Unsupported by Chrome EC, so this bit is never set */
+#define EC_LPC_STATUS_BURST_MODE 0x10
+/* SCI event is pending (requesting SCI query) */
+#define EC_LPC_STATUS_SCI_PENDING 0x20
+/* SMI event is pending (requesting SMI query) */
+#define EC_LPC_STATUS_SMI_PENDING 0x40
+/* (reserved) */
+#define EC_LPC_STATUS_RESERVED 0x80
+
+/*
+ * EC is busy. This covers both the EC processing a command, and the host has
+ * written a new command but the EC hasn't picked it up yet.
+ */
+#define EC_LPC_STATUS_BUSY_MASK \
+ (EC_LPC_STATUS_FROM_HOST | EC_LPC_STATUS_PROCESSING)
+
+/* Host command response codes */
+enum ec_status {
+ EC_RES_SUCCESS = 0,
+ EC_RES_INVALID_COMMAND = 1,
+ EC_RES_ERROR = 2,
+ EC_RES_INVALID_PARAM = 3,
+ EC_RES_ACCESS_DENIED = 4,
+ EC_RES_INVALID_RESPONSE = 5,
+ EC_RES_INVALID_VERSION = 6,
+ EC_RES_INVALID_CHECKSUM = 7,
+ EC_RES_IN_PROGRESS = 8, /* Accepted, command in progress */
+ EC_RES_UNAVAILABLE = 9, /* No response available */
+ EC_RES_TIMEOUT = 10, /* We got a timeout */
+ EC_RES_OVERFLOW = 11, /* Table / data overflow */
+};
+
+/*
+ * Host event codes. Note these are 1-based, not 0-based, because ACPI query
+ * EC command uses code 0 to mean "no event pending". We explicitly specify
+ * each value in the enum listing so they won't change if we delete/insert an
+ * item or rearrange the list (it needs to be stable across platforms, not
+ * just within a single compiled instance).
+ */
+enum host_event_code {
+ EC_HOST_EVENT_LID_CLOSED = 1,
+ EC_HOST_EVENT_LID_OPEN = 2,
+ EC_HOST_EVENT_POWER_BUTTON = 3,
+ EC_HOST_EVENT_AC_CONNECTED = 4,
+ EC_HOST_EVENT_AC_DISCONNECTED = 5,
+ EC_HOST_EVENT_BATTERY_LOW = 6,
+ EC_HOST_EVENT_BATTERY_CRITICAL = 7,
+ EC_HOST_EVENT_BATTERY = 8,
+ EC_HOST_EVENT_THERMAL_THRESHOLD = 9,
+ EC_HOST_EVENT_THERMAL_OVERLOAD = 10,
+ EC_HOST_EVENT_THERMAL = 11,
+ EC_HOST_EVENT_USB_CHARGER = 12,
+ EC_HOST_EVENT_KEY_PRESSED = 13,
+ /*
+ * EC has finished initializing the host interface. The host can check
+ * for this event following sending a EC_CMD_REBOOT_EC command to
+ * determine when the EC is ready to accept subsequent commands.
+ */
+ EC_HOST_EVENT_INTERFACE_READY = 14,
+ /* Keyboard recovery combo has been pressed */
+ EC_HOST_EVENT_KEYBOARD_RECOVERY = 15,
+
+ /* Shutdown due to thermal overload */
+ EC_HOST_EVENT_THERMAL_SHUTDOWN = 16,
+ /* Shutdown due to battery level too low */
+ EC_HOST_EVENT_BATTERY_SHUTDOWN = 17,
+
+ /*
+ * The high bit of the event mask is not used as a host event code. If
+ * it reads back as set, then the entire event mask should be
+ * considered invalid by the host. This can happen when reading the
+ * raw event status via EC_MEMMAP_HOST_EVENTS but the LPC interface is
+ * not initialized on the EC, or improperly configured on the host.
+ */
+ EC_HOST_EVENT_INVALID = 32
+};
+/* Host event mask */
+#define EC_HOST_EVENT_MASK(event_code) (1UL << ((event_code) - 1))
+
+/* Arguments at EC_LPC_ADDR_HOST_ARGS */
+struct ec_lpc_host_args {
+ uint8_t flags;
+ uint8_t command_version;
+ uint8_t data_size;
+ /*
+ * Checksum; sum of command + flags + command_version + data_size +
+ * all params/response data bytes.
+ */
+ uint8_t checksum;
+} __packed;
+
+/* Flags for ec_lpc_host_args.flags */
+/*
+ * Args are from host. Data area at EC_LPC_ADDR_HOST_PARAM contains command
+ * params.
+ *
+ * If EC gets a command and this flag is not set, this is an old-style command.
+ * Command version is 0 and params from host are at EC_LPC_ADDR_OLD_PARAM with
+ * unknown length. EC must respond with an old-style response (that is,
+ * withouth setting EC_HOST_ARGS_FLAG_TO_HOST).
+ */
+#define EC_HOST_ARGS_FLAG_FROM_HOST 0x01
+/*
+ * Args are from EC. Data area at EC_LPC_ADDR_HOST_PARAM contains response.
+ *
+ * If EC responds to a command and this flag is not set, this is an old-style
+ * response. Command version is 0 and response data from EC is at
+ * EC_LPC_ADDR_OLD_PARAM with unknown length.
+ */
+#define EC_HOST_ARGS_FLAG_TO_HOST 0x02
+
+/*
+ * Notes on commands:
+ *
+ * Each command is an 8-byte command value. Commands which take params or
+ * return response data specify structs for that data. If no struct is
+ * specified, the command does not input or output data, respectively.
+ * Parameter/response length is implicit in the structs. Some underlying
+ * communication protocols (I2C, SPI) may add length or checksum headers, but
+ * those are implementation-dependent and not defined here.
+ */
+
+/*****************************************************************************/
+/* General / test commands */
+
+/*
+ * Get protocol version, used to deal with non-backward compatible protocol
+ * changes.
+ */
+#define EC_CMD_PROTO_VERSION 0x00
+
+struct ec_response_proto_version {
+ uint32_t version;
+} __packed;
+
+/*
+ * Hello. This is a simple command to test the EC is responsive to
+ * commands.
+ */
+#define EC_CMD_HELLO 0x01
+
+struct ec_params_hello {
+ uint32_t in_data; /* Pass anything here */
+} __packed;
+
+struct ec_response_hello {
+ uint32_t out_data; /* Output will be in_data + 0x01020304 */
+} __packed;
+
+/* Get version number */
+#define EC_CMD_GET_VERSION 0x02
+
+enum ec_current_image {
+ EC_IMAGE_UNKNOWN = 0,
+ EC_IMAGE_RO,
+ EC_IMAGE_RW
+};
+
+struct ec_response_get_version {
+ /* Null-terminated version strings for RO, RW */
+ char version_string_ro[32];
+ char version_string_rw[32];
+ char reserved[32]; /* Was previously RW-B string */
+ uint32_t current_image; /* One of ec_current_image */
+} __packed;
+
+/* Read test */
+#define EC_CMD_READ_TEST 0x03
+
+struct ec_params_read_test {
+ uint32_t offset; /* Starting value for read buffer */
+ uint32_t size; /* Size to read in bytes */
+} __packed;
+
+struct ec_response_read_test {
+ uint32_t data[32];
+} __packed;
+
+/*
+ * Get build information
+ *
+ * Response is null-terminated string.
+ */
+#define EC_CMD_GET_BUILD_INFO 0x04
+
+/* Get chip info */
+#define EC_CMD_GET_CHIP_INFO 0x05
+
+struct ec_response_get_chip_info {
+ /* Null-terminated strings */
+ char vendor[32];
+ char name[32];
+ char revision[32]; /* Mask version */
+} __packed;
+
+/* Get board HW version */
+#define EC_CMD_GET_BOARD_VERSION 0x06
+
+struct ec_response_board_version {
+ uint16_t board_version; /* A monotonously incrementing number. */
+} __packed;
+
+/*
+ * Read memory-mapped data.
+ *
+ * This is an alternate interface to memory-mapped data for bus protocols
+ * which don't support direct-mapped memory - I2C, SPI, etc.
+ *
+ * Response is params.size bytes of data.
+ */
+#define EC_CMD_READ_MEMMAP 0x07
+
+struct ec_params_read_memmap {
+ uint8_t offset; /* Offset in memmap (EC_MEMMAP_*) */
+ uint8_t size; /* Size to read in bytes */
+} __packed;
+
+/* Read versions supported for a command */
+#define EC_CMD_GET_CMD_VERSIONS 0x08
+
+struct ec_params_get_cmd_versions {
+ uint8_t cmd; /* Command to check */
+} __packed;
+
+struct ec_response_get_cmd_versions {
+ /*
+ * Mask of supported versions; use EC_VER_MASK() to compare with a
+ * desired version.
+ */
+ uint32_t version_mask;
+} __packed;
+
+/*
+ * Check EC communcations status (busy). This is needed on i2c/spi but not
+ * on lpc since it has its own out-of-band busy indicator.
+ *
+ * lpc must read the status from the command register. Attempting this on
+ * lpc will overwrite the args/parameter space and corrupt its data.
+ */
+#define EC_CMD_GET_COMMS_STATUS 0x09
+
+/* Avoid using ec_status which is for return values */
+enum ec_comms_status {
+ EC_COMMS_STATUS_PROCESSING = 1 << 0, /* Processing cmd */
+};
+
+struct ec_response_get_comms_status {
+ uint32_t flags; /* Mask of enum ec_comms_status */
+} __packed;
+
+
+/*****************************************************************************/
+/* Flash commands */
+
+/* Get flash info */
+#define EC_CMD_FLASH_INFO 0x10
+
+struct ec_response_flash_info {
+ /* Usable flash size, in bytes */
+ uint32_t flash_size;
+ /*
+ * Write block size. Write offset and size must be a multiple
+ * of this.
+ */
+ uint32_t write_block_size;
+ /*
+ * Erase block size. Erase offset and size must be a multiple
+ * of this.
+ */
+ uint32_t erase_block_size;
+ /*
+ * Protection block size. Protection offset and size must be a
+ * multiple of this.
+ */
+ uint32_t protect_block_size;
+} __packed;
+
+/*
+ * Read flash
+ *
+ * Response is params.size bytes of data.
+ */
+#define EC_CMD_FLASH_READ 0x11
+
+struct ec_params_flash_read {
+ uint32_t offset; /* Byte offset to read */
+ uint32_t size; /* Size to read in bytes */
+} __packed;
+
+/* Write flash */
+#define EC_CMD_FLASH_WRITE 0x12
+
+struct ec_params_flash_write {
+ uint32_t offset; /* Byte offset to write */
+ uint32_t size; /* Size to write in bytes */
+ /*
+ * Data to write. Could really use EC_PARAM_SIZE - 8, but tidiest to
+ * use a power of 2 so writes stay aligned.
+ */
+ uint8_t data[64];
+} __packed;
+
+/* Erase flash */
+#define EC_CMD_FLASH_ERASE 0x13
+
+struct ec_params_flash_erase {
+ uint32_t offset; /* Byte offset to erase */
+ uint32_t size; /* Size to erase in bytes */
+} __packed;
+
+/*
+ * Get/set flash protection.
+ *
+ * If mask!=0, sets/clear the requested bits of flags. Depending on the
+ * firmware write protect GPIO, not all flags will take effect immediately;
+ * some flags require a subsequent hard reset to take effect. Check the
+ * returned flags bits to see what actually happened.
+ *
+ * If mask=0, simply returns the current flags state.
+ */
+#define EC_CMD_FLASH_PROTECT 0x15
+#define EC_VER_FLASH_PROTECT 1 /* Command version 1 */
+
+/* Flags for flash protection */
+/* RO flash code protected when the EC boots */
+#define EC_FLASH_PROTECT_RO_AT_BOOT (1 << 0)
+/*
+ * RO flash code protected now. If this bit is set, at-boot status cannot
+ * be changed.
+ */
+#define EC_FLASH_PROTECT_RO_NOW (1 << 1)
+/* Entire flash code protected now, until reboot. */
+#define EC_FLASH_PROTECT_ALL_NOW (1 << 2)
+/* Flash write protect GPIO is asserted now */
+#define EC_FLASH_PROTECT_GPIO_ASSERTED (1 << 3)
+/* Error - at least one bank of flash is stuck locked, and cannot be unlocked */
+#define EC_FLASH_PROTECT_ERROR_STUCK (1 << 4)
+/*
+ * Error - flash protection is in inconsistent state. At least one bank of
+ * flash which should be protected is not protected. Usually fixed by
+ * re-requesting the desired flags, or by a hard reset if that fails.
+ */
+#define EC_FLASH_PROTECT_ERROR_INCONSISTENT (1 << 5)
+/* Entile flash code protected when the EC boots */
+#define EC_FLASH_PROTECT_ALL_AT_BOOT (1 << 6)
+
+struct ec_params_flash_protect {
+ uint32_t mask; /* Bits in flags to apply */
+ uint32_t flags; /* New flags to apply */
+} __packed;
+
+struct ec_response_flash_protect {
+ /* Current value of flash protect flags */
+ uint32_t flags;
+ /*
+ * Flags which are valid on this platform. This allows the caller
+ * to distinguish between flags which aren't set vs. flags which can't
+ * be set on this platform.
+ */
+ uint32_t valid_flags;
+ /* Flags which can be changed given the current protection state */
+ uint32_t writable_flags;
+} __packed;
+
+/*
+ * Note: commands 0x14 - 0x19 version 0 were old commands to get/set flash
+ * write protect. These commands may be reused with version > 0.
+ */
+
+/* Get the region offset/size */
+#define EC_CMD_FLASH_REGION_INFO 0x16
+#define EC_VER_FLASH_REGION_INFO 1
+
+enum ec_flash_region {
+ /* Region which holds read-only EC image */
+ EC_FLASH_REGION_RO,
+ /* Region which holds rewritable EC image */
+ EC_FLASH_REGION_RW,
+ /*
+ * Region which should be write-protected in the factory (a superset of
+ * EC_FLASH_REGION_RO)
+ */
+ EC_FLASH_REGION_WP_RO,
+};
+
+struct ec_params_flash_region_info {
+ uint32_t region; /* enum ec_flash_region */
+} __packed;
+
+struct ec_response_flash_region_info {
+ uint32_t offset;
+ uint32_t size;
+} __packed;
+
+/* Read/write VbNvContext */
+#define EC_CMD_VBNV_CONTEXT 0x17
+#define EC_VER_VBNV_CONTEXT 1
+#define EC_VBNV_BLOCK_SIZE 16
+
+enum ec_vbnvcontext_op {
+ EC_VBNV_CONTEXT_OP_READ,
+ EC_VBNV_CONTEXT_OP_WRITE,
+};
+
+struct ec_params_vbnvcontext {
+ uint32_t op;
+ uint8_t block[EC_VBNV_BLOCK_SIZE];
+} __packed;
+
+struct ec_response_vbnvcontext {
+ uint8_t block[EC_VBNV_BLOCK_SIZE];
+} __packed;
+
+/*****************************************************************************/
+/* PWM commands */
+
+/* Get fan target RPM */
+#define EC_CMD_PWM_GET_FAN_TARGET_RPM 0x20
+
+struct ec_response_pwm_get_fan_rpm {
+ uint32_t rpm;
+} __packed;
+
+/* Set target fan RPM */
+#define EC_CMD_PWM_SET_FAN_TARGET_RPM 0x21
+
+struct ec_params_pwm_set_fan_target_rpm {
+ uint32_t rpm;
+} __packed;
+
+/* Get keyboard backlight */
+#define EC_CMD_PWM_GET_KEYBOARD_BACKLIGHT 0x22
+
+struct ec_response_pwm_get_keyboard_backlight {
+ uint8_t percent;
+ uint8_t enabled;
+} __packed;
+
+/* Set keyboard backlight */
+#define EC_CMD_PWM_SET_KEYBOARD_BACKLIGHT 0x23
+
+struct ec_params_pwm_set_keyboard_backlight {
+ uint8_t percent;
+} __packed;
+
+/* Set target fan PWM duty cycle */
+#define EC_CMD_PWM_SET_FAN_DUTY 0x24
+
+struct ec_params_pwm_set_fan_duty {
+ uint32_t percent;
+} __packed;
+
+/*****************************************************************************/
+/*
+ * Lightbar commands. This looks worse than it is. Since we only use one HOST
+ * command to say "talk to the lightbar", we put the "and tell it to do X" part
+ * into a subcommand. We'll make separate structs for subcommands with
+ * different input args, so that we know how much to expect.
+ */
+#define EC_CMD_LIGHTBAR_CMD 0x28
+
+struct rgb_s {
+ uint8_t r, g, b;
+};
+
+#define LB_BATTERY_LEVELS 4
+/* List of tweakable parameters. NOTE: It's __packed so it can be sent in a
+ * host command, but the alignment is the same regardless. Keep it that way.
+ */
+struct lightbar_params {
+ /* Timing */
+ int google_ramp_up;
+ int google_ramp_down;
+ int s3s0_ramp_up;
+ int s0_tick_delay[2]; /* AC=0/1 */
+ int s0a_tick_delay[2]; /* AC=0/1 */
+ int s0s3_ramp_down;
+ int s3_sleep_for;
+ int s3_ramp_up;
+ int s3_ramp_down;
+
+ /* Oscillation */
+ uint8_t new_s0;
+ uint8_t osc_min[2]; /* AC=0/1 */
+ uint8_t osc_max[2]; /* AC=0/1 */
+ uint8_t w_ofs[2]; /* AC=0/1 */
+
+ /* Brightness limits based on the backlight and AC. */
+ uint8_t bright_bl_off_fixed[2]; /* AC=0/1 */
+ uint8_t bright_bl_on_min[2]; /* AC=0/1 */
+ uint8_t bright_bl_on_max[2]; /* AC=0/1 */
+
+ /* Battery level thresholds */
+ uint8_t battery_threshold[LB_BATTERY_LEVELS - 1];
+
+ /* Map [AC][battery_level] to color index */
+ uint8_t s0_idx[2][LB_BATTERY_LEVELS]; /* AP is running */
+ uint8_t s3_idx[2][LB_BATTERY_LEVELS]; /* AP is sleeping */
+
+ /* Color palette */
+ struct rgb_s color[8]; /* 0-3 are Google colors */
+} __packed;
+
+struct ec_params_lightbar {
+ uint8_t cmd; /* Command (see enum lightbar_command) */
+ union {
+ struct {
+ /* no args */
+ } dump, off, on, init, get_seq, get_params;
+
+ struct num {
+ uint8_t num;
+ } brightness, seq, demo;
+
+ struct reg {
+ uint8_t ctrl, reg, value;
+ } reg;
+
+ struct rgb {
+ uint8_t led, red, green, blue;
+ } rgb;
+
+ struct lightbar_params set_params;
+ };
+} __packed;
+
+struct ec_response_lightbar {
+ union {
+ struct dump {
+ struct {
+ uint8_t reg;
+ uint8_t ic0;
+ uint8_t ic1;
+ } vals[23];
+ } dump;
+
+ struct get_seq {
+ uint8_t num;
+ } get_seq;
+
+ struct lightbar_params get_params;
+
+ struct {
+ /* no return params */
+ } off, on, init, brightness, seq, reg, rgb, demo, set_params;
+ };
+} __packed;
+
+/* Lightbar commands */
+enum lightbar_command {
+ LIGHTBAR_CMD_DUMP = 0,
+ LIGHTBAR_CMD_OFF = 1,
+ LIGHTBAR_CMD_ON = 2,
+ LIGHTBAR_CMD_INIT = 3,
+ LIGHTBAR_CMD_BRIGHTNESS = 4,
+ LIGHTBAR_CMD_SEQ = 5,
+ LIGHTBAR_CMD_REG = 6,
+ LIGHTBAR_CMD_RGB = 7,
+ LIGHTBAR_CMD_GET_SEQ = 8,
+ LIGHTBAR_CMD_DEMO = 9,
+ LIGHTBAR_CMD_GET_PARAMS = 10,
+ LIGHTBAR_CMD_SET_PARAMS = 11,
+ LIGHTBAR_NUM_CMDS
+};
+
+/*****************************************************************************/
+/* Verified boot commands */
+
+/*
+ * Note: command code 0x29 version 0 was VBOOT_CMD in Link EVT; it may be
+ * reused for other purposes with version > 0.
+ */
+
+/* Verified boot hash command */
+#define EC_CMD_VBOOT_HASH 0x2A
+
+struct ec_params_vboot_hash {
+ uint8_t cmd; /* enum ec_vboot_hash_cmd */
+ uint8_t hash_type; /* enum ec_vboot_hash_type */
+ uint8_t nonce_size; /* Nonce size; may be 0 */
+ uint8_t reserved0; /* Reserved; set 0 */
+ uint32_t offset; /* Offset in flash to hash */
+ uint32_t size; /* Number of bytes to hash */
+ uint8_t nonce_data[64]; /* Nonce data; ignored if nonce_size=0 */
+} __packed;
+
+struct ec_response_vboot_hash {
+ uint8_t status; /* enum ec_vboot_hash_status */
+ uint8_t hash_type; /* enum ec_vboot_hash_type */
+ uint8_t digest_size; /* Size of hash digest in bytes */
+ uint8_t reserved0; /* Ignore; will be 0 */
+ uint32_t offset; /* Offset in flash which was hashed */
+ uint32_t size; /* Number of bytes hashed */
+ uint8_t hash_digest[64]; /* Hash digest data */
+} __packed;
+
+enum ec_vboot_hash_cmd {
+ EC_VBOOT_HASH_GET = 0, /* Get current hash status */
+ EC_VBOOT_HASH_ABORT = 1, /* Abort calculating current hash */
+ EC_VBOOT_HASH_START = 2, /* Start computing a new hash */
+ EC_VBOOT_HASH_RECALC = 3, /* Synchronously compute a new hash */
+};
+
+enum ec_vboot_hash_type {
+ EC_VBOOT_HASH_TYPE_SHA256 = 0, /* SHA-256 */
+};
+
+enum ec_vboot_hash_status {
+ EC_VBOOT_HASH_STATUS_NONE = 0, /* No hash (not started, or aborted) */
+ EC_VBOOT_HASH_STATUS_DONE = 1, /* Finished computing a hash */
+ EC_VBOOT_HASH_STATUS_BUSY = 2, /* Busy computing a hash */
+};
+
+/*
+ * Special values for offset for EC_VBOOT_HASH_START and EC_VBOOT_HASH_RECALC.
+ * If one of these is specified, the EC will automatically update offset and
+ * size to the correct values for the specified image (RO or RW).
+ */
+#define EC_VBOOT_HASH_OFFSET_RO 0xfffffffe
+#define EC_VBOOT_HASH_OFFSET_RW 0xfffffffd
+
+/*****************************************************************************/
+/* USB charging control commands */
+
+/* Set USB port charging mode */
+#define EC_CMD_USB_CHARGE_SET_MODE 0x30
+
+struct ec_params_usb_charge_set_mode {
+ uint8_t usb_port_id;
+ uint8_t mode;
+} __packed;
+
+/*****************************************************************************/
+/* Persistent storage for host */
+
+/* Maximum bytes that can be read/written in a single command */
+#define EC_PSTORE_SIZE_MAX 64
+
+/* Get persistent storage info */
+#define EC_CMD_PSTORE_INFO 0x40
+
+struct ec_response_pstore_info {
+ /* Persistent storage size, in bytes */
+ uint32_t pstore_size;
+ /* Access size; read/write offset and size must be a multiple of this */
+ uint32_t access_size;
+} __packed;
+
+/*
+ * Read persistent storage
+ *
+ * Response is params.size bytes of data.
+ */
+#define EC_CMD_PSTORE_READ 0x41
+
+struct ec_params_pstore_read {
+ uint32_t offset; /* Byte offset to read */
+ uint32_t size; /* Size to read in bytes */
+} __packed;
+
+/* Write persistent storage */
+#define EC_CMD_PSTORE_WRITE 0x42
+
+struct ec_params_pstore_write {
+ uint32_t offset; /* Byte offset to write */
+ uint32_t size; /* Size to write in bytes */
+ uint8_t data[EC_PSTORE_SIZE_MAX];
+} __packed;
+
+/*****************************************************************************/
+/* Real-time clock */
+
+/* RTC params and response structures */
+struct ec_params_rtc {
+ uint32_t time;
+} __packed;
+
+struct ec_response_rtc {
+ uint32_t time;
+} __packed;
+
+/* These use ec_response_rtc */
+#define EC_CMD_RTC_GET_VALUE 0x44
+#define EC_CMD_RTC_GET_ALARM 0x45
+
+/* These all use ec_params_rtc */
+#define EC_CMD_RTC_SET_VALUE 0x46
+#define EC_CMD_RTC_SET_ALARM 0x47
+
+/*****************************************************************************/
+/* Port80 log access */
+
+/* Get last port80 code from previous boot */
+#define EC_CMD_PORT80_LAST_BOOT 0x48
+
+struct ec_response_port80_last_boot {
+ uint16_t code;
+} __packed;
+
+/*****************************************************************************/
+/* Thermal engine commands */
+
+/* Set thershold value */
+#define EC_CMD_THERMAL_SET_THRESHOLD 0x50
+
+struct ec_params_thermal_set_threshold {
+ uint8_t sensor_type;
+ uint8_t threshold_id;
+ uint16_t value;
+} __packed;
+
+/* Get threshold value */
+#define EC_CMD_THERMAL_GET_THRESHOLD 0x51
+
+struct ec_params_thermal_get_threshold {
+ uint8_t sensor_type;
+ uint8_t threshold_id;
+} __packed;
+
+struct ec_response_thermal_get_threshold {
+ uint16_t value;
+} __packed;
+
+/* Toggle automatic fan control */
+#define EC_CMD_THERMAL_AUTO_FAN_CTRL 0x52
+
+/* Get TMP006 calibration data */
+#define EC_CMD_TMP006_GET_CALIBRATION 0x53
+
+struct ec_params_tmp006_get_calibration {
+ uint8_t index;
+} __packed;
+
+struct ec_response_tmp006_get_calibration {
+ float s0;
+ float b0;
+ float b1;
+ float b2;
+} __packed;
+
+/* Set TMP006 calibration data */
+#define EC_CMD_TMP006_SET_CALIBRATION 0x54
+
+struct ec_params_tmp006_set_calibration {
+ uint8_t index;
+ uint8_t reserved[3]; /* Reserved; set 0 */
+ float s0;
+ float b0;
+ float b1;
+ float b2;
+} __packed;
+
+/*****************************************************************************/
+/* MKBP - Matrix KeyBoard Protocol */
+
+/*
+ * Read key state
+ *
+ * Returns raw data for keyboard cols; see ec_response_mkbp_info.cols for
+ * expected response size.
+ */
+#define EC_CMD_MKBP_STATE 0x60
+
+/* Provide information about the matrix : number of rows and columns */
+#define EC_CMD_MKBP_INFO 0x61
+
+struct ec_response_mkbp_info {
+ uint32_t rows;
+ uint32_t cols;
+ uint8_t switches;
+} __packed;
+
+/* Simulate key press */
+#define EC_CMD_MKBP_SIMULATE_KEY 0x62
+
+struct ec_params_mkbp_simulate_key {
+ uint8_t col;
+ uint8_t row;
+ uint8_t pressed;
+} __packed;
+
+/* Configure keyboard scanning */
+#define EC_CMD_MKBP_SET_CONFIG 0x64
+#define EC_CMD_MKBP_GET_CONFIG 0x65
+
+/* flags */
+enum mkbp_config_flags {
+ EC_MKBP_FLAGS_ENABLE = 1, /* Enable keyboard scanning */
+};
+
+enum mkbp_config_valid {
+ EC_MKBP_VALID_SCAN_PERIOD = 1 << 0,
+ EC_MKBP_VALID_POLL_TIMEOUT = 1 << 1,
+ EC_MKBP_VALID_MIN_POST_SCAN_DELAY = 1 << 3,
+ EC_MKBP_VALID_OUTPUT_SETTLE = 1 << 4,
+ EC_MKBP_VALID_DEBOUNCE_DOWN = 1 << 5,
+ EC_MKBP_VALID_DEBOUNCE_UP = 1 << 6,
+ EC_MKBP_VALID_FIFO_MAX_DEPTH = 1 << 7,
+};
+
+/* Configuration for our key scanning algorithm */
+struct ec_mkbp_config {
+ uint32_t valid_mask; /* valid fields */
+ uint8_t flags; /* some flags (enum mkbp_config_flags) */
+ uint8_t valid_flags; /* which flags are valid */
+ uint16_t scan_period_us; /* period between start of scans */
+ /* revert to interrupt mode after no activity for this long */
+ uint32_t poll_timeout_us;
+ /*
+ * minimum post-scan relax time. Once we finish a scan we check
+ * the time until we are due to start the next one. If this time is
+ * shorter this field, we use this instead.
+ */
+ uint16_t min_post_scan_delay_us;
+ /* delay between setting up output and waiting for it to settle */
+ uint16_t output_settle_us;
+ uint16_t debounce_down_us; /* time for debounce on key down */
+ uint16_t debounce_up_us; /* time for debounce on key up */
+ /* maximum depth to allow for fifo (0 = no keyscan output) */
+ uint8_t fifo_max_depth;
+} __packed;
+
+struct ec_params_mkbp_set_config {
+ struct ec_mkbp_config config;
+} __packed;
+
+struct ec_response_mkbp_get_config {
+ struct ec_mkbp_config config;
+} __packed;
+
+/* Run the key scan emulation */
+#define EC_CMD_KEYSCAN_SEQ_CTRL 0x66
+
+enum ec_keyscan_seq_cmd {
+ EC_KEYSCAN_SEQ_STATUS = 0, /* Get status information */
+ EC_KEYSCAN_SEQ_CLEAR = 1, /* Clear sequence */
+ EC_KEYSCAN_SEQ_ADD = 2, /* Add item to sequence */
+ EC_KEYSCAN_SEQ_START = 3, /* Start running sequence */
+ EC_KEYSCAN_SEQ_COLLECT = 4, /* Collect sequence summary data */
+};
+
+enum ec_collect_flags {
+ /*
+ * Indicates this scan was processed by the EC. Due to timing, some
+ * scans may be skipped.
+ */
+ EC_KEYSCAN_SEQ_FLAG_DONE = 1 << 0,
+};
+
+struct ec_collect_item {
+ uint8_t flags; /* some flags (enum ec_collect_flags) */
+};
+
+struct ec_params_keyscan_seq_ctrl {
+ uint8_t cmd; /* Command to send (enum ec_keyscan_seq_cmd) */
+ union {
+ struct {
+ uint8_t active; /* still active */
+ uint8_t num_items; /* number of items */
+ /* Current item being presented */
+ uint8_t cur_item;
+ } status;
+ struct {
+ /*
+ * Absolute time for this scan, measured from the
+ * start of the sequence.
+ */
+ uint32_t time_us;
+ uint8_t scan[0]; /* keyscan data */
+ } add;
+ struct {
+ uint8_t start_item; /* First item to return */
+ uint8_t num_items; /* Number of items to return */
+ } collect;
+ };
+} __packed;
+
+struct ec_result_keyscan_seq_ctrl {
+ union {
+ struct {
+ uint8_t num_items; /* Number of items */
+ /* Data for each item */
+ struct ec_collect_item item[0];
+ } collect;
+ };
+} __packed;
+
+/*****************************************************************************/
+/* Temperature sensor commands */
+
+/* Read temperature sensor info */
+#define EC_CMD_TEMP_SENSOR_GET_INFO 0x70
+
+struct ec_params_temp_sensor_get_info {
+ uint8_t id;
+} __packed;
+
+struct ec_response_temp_sensor_get_info {
+ char sensor_name[32];
+ uint8_t sensor_type;
+} __packed;
+
+/*****************************************************************************/
+
+/*
+ * Note: host commands 0x80 - 0x87 are reserved to avoid conflict with ACPI
+ * commands accidentally sent to the wrong interface. See the ACPI section
+ * below.
+ */
+
+/*****************************************************************************/
+/* Host event commands */
+
+/*
+ * Host event mask params and response structures, shared by all of the host
+ * event commands below.
+ */
+struct ec_params_host_event_mask {
+ uint32_t mask;
+} __packed;
+
+struct ec_response_host_event_mask {
+ uint32_t mask;
+} __packed;
+
+/* These all use ec_response_host_event_mask */
+#define EC_CMD_HOST_EVENT_GET_B 0x87
+#define EC_CMD_HOST_EVENT_GET_SMI_MASK 0x88
+#define EC_CMD_HOST_EVENT_GET_SCI_MASK 0x89
+#define EC_CMD_HOST_EVENT_GET_WAKE_MASK 0x8d
+
+/* These all use ec_params_host_event_mask */
+#define EC_CMD_HOST_EVENT_SET_SMI_MASK 0x8a
+#define EC_CMD_HOST_EVENT_SET_SCI_MASK 0x8b
+#define EC_CMD_HOST_EVENT_CLEAR 0x8c
+#define EC_CMD_HOST_EVENT_SET_WAKE_MASK 0x8e
+#define EC_CMD_HOST_EVENT_CLEAR_B 0x8f
+
+/*****************************************************************************/
+/* Switch commands */
+
+/* Enable/disable LCD backlight */
+#define EC_CMD_SWITCH_ENABLE_BKLIGHT 0x90
+
+struct ec_params_switch_enable_backlight {
+ uint8_t enabled;
+} __packed;
+
+/* Enable/disable WLAN/Bluetooth */
+#define EC_CMD_SWITCH_ENABLE_WIRELESS 0x91
+
+struct ec_params_switch_enable_wireless {
+ uint8_t enabled;
+} __packed;
+
+/*****************************************************************************/
+/* GPIO commands. Only available on EC if write protect has been disabled. */
+
+/* Set GPIO output value */
+#define EC_CMD_GPIO_SET 0x92
+
+struct ec_params_gpio_set {
+ char name[32];
+ uint8_t val;
+} __packed;
+
+/* Get GPIO value */
+#define EC_CMD_GPIO_GET 0x93
+
+struct ec_params_gpio_get {
+ char name[32];
+} __packed;
+struct ec_response_gpio_get {
+ uint8_t val;
+} __packed;
+
+/*****************************************************************************/
+/* I2C commands. Only available when flash write protect is unlocked. */
+
+/* Read I2C bus */
+#define EC_CMD_I2C_READ 0x94
+
+struct ec_params_i2c_read {
+ uint16_t addr;
+ uint8_t read_size; /* Either 8 or 16. */
+ uint8_t port;
+ uint8_t offset;
+} __packed;
+struct ec_response_i2c_read {
+ uint16_t data;
+} __packed;
+
+/* Write I2C bus */
+#define EC_CMD_I2C_WRITE 0x95
+
+struct ec_params_i2c_write {
+ uint16_t data;
+ uint16_t addr;
+ uint8_t write_size; /* Either 8 or 16. */
+ uint8_t port;
+ uint8_t offset;
+} __packed;
+
+/*****************************************************************************/
+/* Charge state commands. Only available when flash write protect unlocked. */
+
+/* Force charge state machine to stop in idle mode */
+#define EC_CMD_CHARGE_FORCE_IDLE 0x96
+
+struct ec_params_force_idle {
+ uint8_t enabled;
+} __packed;
+
+/*****************************************************************************/
+/* Console commands. Only available when flash write protect is unlocked. */
+
+/* Snapshot console output buffer for use by EC_CMD_CONSOLE_READ. */
+#define EC_CMD_CONSOLE_SNAPSHOT 0x97
+
+/*
+ * Read next chunk of data from saved snapshot.
+ *
+ * Response is null-terminated string. Empty string, if there is no more
+ * remaining output.
+ */
+#define EC_CMD_CONSOLE_READ 0x98
+
+/*****************************************************************************/
+
+/*
+ * Cut off battery power output if the battery supports.
+ *
+ * For unsupported battery, just don't implement this command and lets EC
+ * return EC_RES_INVALID_COMMAND.
+ */
+#define EC_CMD_BATTERY_CUT_OFF 0x99
+
+/*****************************************************************************/
+/* Temporary debug commands. TODO: remove this crosbug.com/p/13849 */
+
+/*
+ * Dump charge state machine context.
+ *
+ * Response is a binary dump of charge state machine context.
+ */
+#define EC_CMD_CHARGE_DUMP 0xa0
+
+/*
+ * Set maximum battery charging current.
+ */
+#define EC_CMD_CHARGE_CURRENT_LIMIT 0xa1
+
+struct ec_params_current_limit {
+ uint32_t limit;
+} __packed;
+
+/*****************************************************************************/
+/* System commands */
+
+/*
+ * TODO: this is a confusing name, since it doesn't necessarily reboot the EC.
+ * Rename to "set image" or something similar.
+ */
+#define EC_CMD_REBOOT_EC 0xd2
+
+/* Command */
+enum ec_reboot_cmd {
+ EC_REBOOT_CANCEL = 0, /* Cancel a pending reboot */
+ EC_REBOOT_JUMP_RO = 1, /* Jump to RO without rebooting */
+ EC_REBOOT_JUMP_RW = 2, /* Jump to RW without rebooting */
+ /* (command 3 was jump to RW-B) */
+ EC_REBOOT_COLD = 4, /* Cold-reboot */
+ EC_REBOOT_DISABLE_JUMP = 5, /* Disable jump until next reboot */
+ EC_REBOOT_HIBERNATE = 6 /* Hibernate EC */
+};
+
+/* Flags for ec_params_reboot_ec.reboot_flags */
+#define EC_REBOOT_FLAG_RESERVED0 (1 << 0) /* Was recovery request */
+#define EC_REBOOT_FLAG_ON_AP_SHUTDOWN (1 << 1) /* Reboot after AP shutdown */
+
+struct ec_params_reboot_ec {
+ uint8_t cmd; /* enum ec_reboot_cmd */
+ uint8_t flags; /* See EC_REBOOT_FLAG_* */
+} __packed;
+
+/*
+ * Get information on last EC panic.
+ *
+ * Returns variable-length platform-dependent panic information. See panic.h
+ * for details.
+ */
+#define EC_CMD_GET_PANIC_INFO 0xd3
+
+/*****************************************************************************/
+/*
+ * ACPI commands
+ *
+ * These are valid ONLY on the ACPI command/data port.
+ */
+
+/*
+ * ACPI Read Embedded Controller
+ *
+ * This reads from ACPI memory space on the EC (EC_ACPI_MEM_*).
+ *
+ * Use the following sequence:
+ *
+ * - Write EC_CMD_ACPI_READ to EC_LPC_ADDR_ACPI_CMD
+ * - Wait for EC_LPC_CMDR_PENDING bit to clear
+ * - Write address to EC_LPC_ADDR_ACPI_DATA
+ * - Wait for EC_LPC_CMDR_DATA bit to set
+ * - Read value from EC_LPC_ADDR_ACPI_DATA
+ */
+#define EC_CMD_ACPI_READ 0x80
+
+/*
+ * ACPI Write Embedded Controller
+ *
+ * This reads from ACPI memory space on the EC (EC_ACPI_MEM_*).
+ *
+ * Use the following sequence:
+ *
+ * - Write EC_CMD_ACPI_WRITE to EC_LPC_ADDR_ACPI_CMD
+ * - Wait for EC_LPC_CMDR_PENDING bit to clear
+ * - Write address to EC_LPC_ADDR_ACPI_DATA
+ * - Wait for EC_LPC_CMDR_PENDING bit to clear
+ * - Write value to EC_LPC_ADDR_ACPI_DATA
+ */
+#define EC_CMD_ACPI_WRITE 0x81
+
+/*
+ * ACPI Query Embedded Controller
+ *
+ * This clears the lowest-order bit in the currently pending host events, and
+ * sets the result code to the 1-based index of the bit (event 0x00000001 = 1,
+ * event 0x80000000 = 32), or 0 if no event was pending.
+ */
+#define EC_CMD_ACPI_QUERY_EVENT 0x84
+
+/* Valid addresses in ACPI memory space, for read/write commands */
+/* Memory space version; set to EC_ACPI_MEM_VERSION_CURRENT */
+#define EC_ACPI_MEM_VERSION 0x00
+/*
+ * Test location; writing value here updates test compliment byte to (0xff -
+ * value).
+ */
+#define EC_ACPI_MEM_TEST 0x01
+/* Test compliment; writes here are ignored. */
+#define EC_ACPI_MEM_TEST_COMPLIMENT 0x02
+/* Keyboard backlight brightness percent (0 - 100) */
+#define EC_ACPI_MEM_KEYBOARD_BACKLIGHT 0x03
+
+/* Current version of ACPI memory address space */
+#define EC_ACPI_MEM_VERSION_CURRENT 1
+
+
+/*****************************************************************************/
+/*
+ * Special commands
+ *
+ * These do not follow the normal rules for commands. See each command for
+ * details.
+ */
+
+/*
+ * Reboot NOW
+ *
+ * This command will work even when the EC LPC interface is busy, because the
+ * reboot command is processed at interrupt level. Note that when the EC
+ * reboots, the host will reboot too, so there is no response to this command.
+ *
+ * Use EC_CMD_REBOOT_EC to reboot the EC more politely.
+ */
+#define EC_CMD_REBOOT 0xd1 /* Think "die" */
+
+/*
+ * Resend last response (not supported on LPC).
+ *
+ * Returns EC_RES_UNAVAILABLE if there is no response available - for example,
+ * there was no previous command, or the previous command's response was too
+ * big to save.
+ */
+#define EC_CMD_RESEND_RESPONSE 0xdb
+
+/*
+ * This header byte on a command indicate version 0. Any header byte less
+ * than this means that we are talking to an old EC which doesn't support
+ * versioning. In that case, we assume version 0.
+ *
+ * Header bytes greater than this indicate a later version. For example,
+ * EC_CMD_VERSION0 + 1 means we are using version 1.
+ *
+ * The old EC interface must not use commands 0dc or higher.
+ */
+#define EC_CMD_VERSION0 0xdc
+
+#endif /* !__ACPI__ */
+
+#endif /* __CROS_EC_COMMANDS_H */
--
1.8.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 2/6] mfd: Add ChromeOS EC implementation
2013-02-13 2:42 [PATCH v2 0/6] Add ChromeOS Embedded Controller support Simon Glass
2013-02-13 2:42 ` [PATCH v2 1/6] mfd: Add ChromeOS EC messages header Simon Glass
@ 2013-02-13 2:42 ` Simon Glass
2013-02-13 3:35 ` Joe Perches
2013-02-13 2:42 ` [PATCH v2 3/6] mfd: Add ChromeOS EC I2C driver Simon Glass
` (3 subsequent siblings)
5 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2013-02-13 2:42 UTC (permalink / raw)
To: LKML
Cc: Samuel Ortiz, Simon Glass, Che-Liang Chiou, Jonathan Kliegman,
Luigi Semenzato, Olof Johansson, Vincent Palatin, Grant Likely,
Rob Herring, Rob Landley, devicetree-discuss, linux-doc
This is the base EC implementation, which provides a high level
interface to the EC for use by the rest of the kernel. The actual
communcations is dealt with by a separate protocol driver which
registers itself with this interface.
Interrupts are passed on through a notifier. The driver supports
resume notification also, in case drivers wish to perform some
action there.
A simple message structure is used to pass messages to the
protocol driver.
Signed-off-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Che-Liang Chiou <clchiou@chromium.org>
Signed-off-by: Jonathan Kliegman <kliegs@chromium.org>
Signed-off-by: Luigi Semenzato <semenzato@chromium.org>
Signed-off-by: Olof Johansson <olofj@chromium.org>
Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
---
Changes in v2:
- Remove use of __devinit/__devexit
Documentation/devicetree/bindings/mfd/cros-ec.txt | 56 ++++++
drivers/mfd/Kconfig | 8 +
drivers/mfd/Makefile | 1 +
drivers/mfd/cros_ec.c | 219 ++++++++++++++++++++++
include/linux/mfd/cros_ec.h | 190 +++++++++++++++++++
5 files changed, 474 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/cros-ec.txt
create mode 100644 drivers/mfd/cros_ec.c
create mode 100644 include/linux/mfd/cros_ec.h
diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
new file mode 100644
index 0000000..e0e59c5
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
@@ -0,0 +1,56 @@
+ChromeOS Embedded Controller
+
+Google's ChromeOS EC is a Cortex-M device which talks to the AP and
+implements various function such as keyboard and battery charging.
+
+The EC can be connect through various means (I2C, SPI, LPC) and the
+compatible string used depends on the inteface. Each connection method has
+its own driver which connects to the top level interface-agnostic EC driver.
+Other Linux driver (such as cros-ec-keyb for the matrix keyboard) connect to
+the top-level driver.
+
+Required properties (I2C):
+- compatible: "google,cros-ec-i2c"
+- reg: I2C slave address
+
+Required properties (SPI):
+- compatible: "google,cros-ec-spi"
+- reg: SPI chip select
+
+Required properties (LPC):
+- compatible: "google,cros-ec-lpc"
+- reg: List of (IO address, size) pairs defining the interface uses
+
+
+Example for I2C:
+
+i2c@12CA0000 {
+ cros-ec@1e {
+ reg = <0x1e>;
+ compatible = "google,cros-ec-i2c";
+ interrupts = <14 0>;
+ interrupt-parent = <&wakeup_eint>;
+ wakeup-source;
+ };
+
+
+Example for SPI:
+
+spi@131b0000 {
+ ec@0 {
+ compatible = "google,cros-ec-spi";
+ reg = <0x0>;
+ interrupts = <14 0>;
+ interrupt-parent = <&wakeup_eint>;
+ wakeup-source;
+ spi-max-frequency = <5000000>;
+ controller-data {
+ cs-gpio = <&gpf0 3 4 3 0>;
+ samsung,spi-cs;
+ samsung,spi-feedback-delay = <2>;
+ };
+ };
+};
+
+
+Example for LPC is not supplied as it is not yet implemented.
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 671f5b1..837a16b 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -21,6 +21,14 @@ config MFD_88PM860X
select individual components like voltage regulators, RTC and
battery-charger under the corresponding menus.
+config MFD_CROS_EC
+ bool "Support ChromeOS Embedded Controller"
+ help
+ If you say yes here you get support for the ChromeOS Embedded
+ Controller (EC) providing keyboard, battery and power services.
+ You also ned to enable the driver for the bus you are using. The
+ protocol for talking to the EC is defined by the bus driver.
+
config MFD_88PM800
tristate "Support Marvell 88PM800"
depends on I2C=y && GENERIC_HARDIRQS
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index b90409c..967767d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_MFD_88PM800) += 88pm800.o 88pm80x.o
obj-$(CONFIG_MFD_88PM805) += 88pm805.o 88pm80x.o
obj-$(CONFIG_MFD_SM501) += sm501.o
obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
+obj-$(CONFIG_MFD_CROS_EC) += cros_ec.o
rtsx_pci-objs := rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o
obj-$(CONFIG_MFD_RTSX_PCI) += rtsx_pci.o
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
new file mode 100644
index 0000000..fc7fce3
--- /dev/null
+++ b/drivers/mfd/cros_ec.c
@@ -0,0 +1,219 @@
+/*
+ * ChromeOS EC multi-function device
+ *
+ * Copyright (C) 2012 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.
+ *
+ * The ChromeOS EC multi function device is used to mux all the requests
+ * to the EC device for its multiple features: keyboard controller,
+ * battery charging and regulator control, firmware update.
+ */
+
+#include <linux/interrupt.h>
+#include <linux/slab.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+
+int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
+ struct cros_ec_msg *msg)
+{
+ uint8_t *out;
+ int csum, i;
+
+ BUG_ON(msg->out_len > EC_HOST_PARAM_SIZE);
+ out = ec_dev->dout;
+ out[0] = EC_CMD_VERSION0 + msg->version;
+ out[1] = msg->cmd;
+ out[2] = msg->out_len;
+ csum = out[0] + out[1] + out[2];
+ for (i = 0; i < msg->out_len; i++)
+ csum += out[EC_MSG_TX_HEADER_BYTES + i] = msg->out_buf[i];
+ out[EC_MSG_TX_HEADER_BYTES + msg->out_len] = (uint8_t)(csum & 0xff);
+
+ return EC_MSG_TX_PROTO_BYTES + msg->out_len;
+}
+
+static int cros_ec_command_sendrecv(struct cros_ec_device *ec_dev,
+ uint16_t cmd, void *out_buf, int out_len,
+ void *in_buf, int in_len)
+{
+ struct cros_ec_msg msg;
+
+ msg.version = cmd >> 8;
+ msg.cmd = cmd & 0xff;
+ msg.out_buf = out_buf;
+ msg.out_len = out_len;
+ msg.in_buf = in_buf;
+ msg.in_len = in_len;
+
+ return ec_dev->command_xfer(ec_dev, &msg);
+}
+
+static int cros_ec_command_recv(struct cros_ec_device *ec_dev,
+ uint16_t cmd, void *buf, int buf_len)
+{
+ return cros_ec_command_sendrecv(ec_dev, cmd, NULL, 0, buf, buf_len);
+}
+
+static int cros_ec_command_send(struct cros_ec_device *ec_dev,
+ uint16_t cmd, void *buf, int buf_len)
+{
+ return cros_ec_command_sendrecv(ec_dev, cmd, buf, buf_len, NULL, 0);
+}
+
+struct cros_ec_device *cros_ec_alloc(const char *name)
+{
+ struct cros_ec_device *ec_dev;
+
+ ec_dev = kzalloc(sizeof(*ec_dev), GFP_KERNEL);
+ if (ec_dev == NULL) {
+ dev_err(ec_dev->dev, "cannot allocate\n");
+ return NULL;
+ }
+ ec_dev->name = name;
+
+ return ec_dev;
+}
+
+void cros_ec_free(struct cros_ec_device *ec)
+{
+ kfree(ec);
+}
+
+static irqreturn_t ec_irq_thread(int irq, void *data)
+{
+ struct cros_ec_device *ec_dev = data;
+
+ if (device_may_wakeup(ec_dev->dev))
+ pm_wakeup_event(ec_dev->dev, 0);
+
+ blocking_notifier_call_chain(&ec_dev->event_notifier, 1, ec_dev);
+
+ return IRQ_HANDLED;
+}
+
+static struct mfd_cell cros_devs[] = {
+ {
+ .name = "cros-ec-keyb",
+ .id = 1,
+ },
+};
+
+int cros_ec_register(struct cros_ec_device *ec_dev)
+{
+ struct device *dev = ec_dev->dev;
+ int err = 0;
+
+ BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->event_notifier);
+ BLOCKING_INIT_NOTIFIER_HEAD(&ec_dev->wake_notifier);
+
+ ec_dev->command_send = cros_ec_command_send;
+ ec_dev->command_recv = cros_ec_command_recv;
+ ec_dev->command_sendrecv = cros_ec_command_sendrecv;
+
+ if (ec_dev->din_size) {
+ ec_dev->din = kmalloc(ec_dev->din_size, GFP_KERNEL);
+ if (!ec_dev->din) {
+ err = -ENOMEM;
+ dev_err(dev, "cannot allocate din\n");
+ goto fail_din;
+ }
+ }
+ if (ec_dev->dout_size) {
+ ec_dev->dout = kmalloc(ec_dev->dout_size, GFP_KERNEL);
+ if (!ec_dev->dout) {
+ err = -ENOMEM;
+ dev_err(dev, "cannot allocate dout\n");
+ goto fail_dout;
+ }
+ }
+
+ if (!ec_dev->irq) {
+ dev_dbg(dev, "no valid IRQ: %d\n", ec_dev->irq);
+ goto fail_irq;
+ }
+
+ err = request_threaded_irq(ec_dev->irq, NULL, ec_irq_thread,
+ IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+ "chromeos-ec", ec_dev);
+ if (err) {
+ dev_err(dev, "request irq %d: error %d\n", ec_dev->irq, err);
+ goto fail_irq;
+ }
+
+ err = mfd_add_devices(dev, 0, cros_devs,
+ ARRAY_SIZE(cros_devs),
+ NULL, ec_dev->irq, NULL);
+ if (err) {
+ dev_err(dev, "failed to add mfd devices");
+ goto fail_mfd;
+ }
+
+ dev_info(dev, "Chrome EC (%s)\n", ec_dev->name);
+
+ return 0;
+
+fail_mfd:
+ free_irq(ec_dev->irq, ec_dev);
+fail_irq:
+ kfree(ec_dev->dout);
+fail_dout:
+ kfree(ec_dev->din);
+fail_din:
+ return err;
+}
+
+int cros_ec_remove(struct cros_ec_device *ec_dev)
+{
+ mfd_remove_devices(ec_dev->dev);
+ free_irq(ec_dev->irq, ec_dev);
+ kfree(ec_dev->dout);
+ kfree(ec_dev->din);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+int cros_ec_suspend(struct cros_ec_device *ec_dev)
+{
+ struct device *dev = ec_dev->dev;
+
+ if (device_may_wakeup(dev))
+ ec_dev->wake_enabled = !enable_irq_wake(ec_dev->irq);
+
+ disable_irq(ec_dev->irq);
+
+ return 0;
+}
+
+int cros_ec_resume(struct cros_ec_device *ec_dev)
+{
+ /*
+ * When the EC is not a wake source, then it could not have caused the
+ * resume, so we should do the resume processing. This may clear the
+ * EC's key scan buffer, for example. If the EC is a wake source (e.g.
+ * the lid is open and the user might press a key to wake) then we
+ * don't want to do resume processing (key scan buffer should be
+ * preserved).
+ */
+ if (!ec_dev->wake_enabled)
+ blocking_notifier_call_chain(&ec_dev->wake_notifier, 1, ec_dev);
+ enable_irq(ec_dev->irq);
+
+ if (ec_dev->wake_enabled) {
+ disable_irq_wake(ec_dev->irq);
+ ec_dev->wake_enabled = 0;
+ }
+
+ return 0;
+}
+#endif
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
new file mode 100644
index 0000000..3d7cb40
--- /dev/null
+++ b/include/linux/mfd/cros_ec.h
@@ -0,0 +1,190 @@
+/*
+ * ChromeOS EC multi-function device
+ *
+ * Copyright (C) 2012 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.
+ */
+
+#ifndef __LINUX_MFD_CROS_EC_H
+#define __LINUX_MFD_CROS_EC_H
+
+struct i2c_msg;
+
+#include <linux/mfd/cros_ec_commands.h>
+
+/*
+ * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
+ */
+enum {
+ EC_MSG_TX_HEADER_BYTES = 3,
+ EC_MSG_TX_TRAILER_BYTES = 1,
+ EC_MSG_TX_PROTO_BYTES = EC_MSG_TX_HEADER_BYTES +
+ EC_MSG_TX_TRAILER_BYTES,
+ EC_MSG_RX_PROTO_BYTES = 3,
+
+ /* Max length of messages */
+ EC_MSG_BYTES = EC_HOST_PARAM_SIZE + EC_MSG_TX_PROTO_BYTES,
+
+};
+
+/**
+ * struct cros_ec_msg - A message sent to the EC, and its reply
+ *
+ * @version: Command version number (often 0)
+ * @cmd: Command to send (EC_CMD_...)
+ * @out_buf: Outgoing payload (to EC)
+ * @outlen: Outgoing length
+ * @in_buf: Incoming payload (from EC)
+ * @in_len: Incoming length
+ */
+struct cros_ec_msg {
+ u8 version;
+ u8 cmd;
+ uint8_t *out_buf;
+ int out_len;
+ uint8_t *in_buf;
+ int in_len;
+};
+
+/**
+ * struct cros_ec_device - Information about a ChromeOS EC device
+ *
+ * @name: Name of this EC interface
+ * @priv: Private data
+ * @irq: Interrupt to use
+ * @din: input buffer (from EC)
+ * @dout: output buffer (to EC)
+ * \note
+ * These two buffers will always be dword-aligned and include enough
+ * space for up to 7 word-alignment bytes also, so we can ensure that
+ * the body of the message is always dword-aligned (64-bit).
+ *
+ * We use this alignment to keep ARM and x86 happy. Probably word
+ * alignment would be OK, there might be a small performance advantage
+ * to using dword.
+ * @din_size: size of din buffer
+ * @dout_size: size of dout buffer
+ * @command_send: send a command
+ * @command_recv: receive a command
+ * @get_name: return name of EC device (e.g. 'chromeos-ec')
+ * @get_phys_name: return name of physical comms layer (e.g. 'i2c-4')
+ * @get_parent: return pointer to parent device (e.g. i2c or spi device)
+ * @dev: Device pointer
+ * dev_lock: Lock to prevent concurrent access
+ * @wake_enabled: true if this device can wake the system from sleep
+ * @event_notifier: interrupt event notifier for transport devices
+ * @wake_notifier: wake notfier for client devices (e.g. keyboard). This
+ * indicates to sub-drivers that we have woken up from resume but we
+ * were not a wakeup source.
+ */
+struct cros_ec_device {
+ const char *name;
+ void *priv;
+ int irq;
+ uint8_t *din;
+ uint8_t *dout;
+ int din_size;
+ int dout_size;
+ int (*command_send)(struct cros_ec_device *ec,
+ uint16_t cmd, void *out_buf, int out_len);
+ int (*command_recv)(struct cros_ec_device *ec,
+ uint16_t cmd, void *in_buf, int in_len);
+ int (*command_sendrecv)(struct cros_ec_device *ec,
+ uint16_t cmd, void *out_buf, int out_len,
+ void *in_buf, int in_len);
+ int (*command_xfer)(struct cros_ec_device *ec,
+ struct cros_ec_msg *msg);
+
+ const char *(*get_name)(struct cros_ec_device *ec_dev);
+
+ const char *(*get_phys_name)(struct cros_ec_device *ec_dev);
+
+ struct device *(*get_parent)(struct cros_ec_device *ec_dev);
+
+ /* These are --private-- fields - do not assign */
+ struct device *dev;
+ struct mutex dev_lock;
+ bool wake_enabled;
+ struct blocking_notifier_head event_notifier;
+ struct blocking_notifier_head wake_notifier;
+};
+
+/**
+ * cros_ec_suspend - Handle a suspend operation for the ChromeOS EC device
+ *
+ * This can be called by drivers to handle a suspend event.
+ *
+ * ec_dev: Device to suspend
+ * @return 0 if ok, -ve on error
+ */
+int cros_ec_suspend(struct cros_ec_device *ec_dev);
+
+/**
+ * cros_ec_resume - Handle a resume operation for the ChromeOS EC device
+ *
+ * This can be called by drivers to handle a resume event.
+ *
+ * @ec_dev: Device to resume
+ * @return 0 if ok, -ve on error
+ */
+int cros_ec_resume(struct cros_ec_device *ec_dev);
+
+/**
+ * cros_ec_prepare_tx - Prepare an outgoing message in the output buffer
+ *
+ * This is intended to be used by all ChromeOS EC drivers, but at present
+ * only SPI uses it. Once LPC uses the same protocol it can start using it.
+ * I2C could use it now, with a refactor of the existing code.
+ *
+ * @ec_dev: Device to register
+ * @msg: Message to write
+ */
+int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
+ struct cros_ec_msg *msg);
+
+/**
+ * cros_ec_remove - Remove a ChromeOS EC
+ *
+ * Call this to deregister a ChromeOS EC. After this you should call
+ * cros_ec_free().
+ *
+ * @ec_dev: Device to register
+ * @return 0 if ok, -ve on error
+ */
+int cros_ec_remove(struct cros_ec_device *ec_dev);
+
+/**
+ * cros_ec_register - Register a new ChromeOS EC, using the provided info
+ *
+ * Before calling this, call cros_ec_alloc() to get a pointer to a new device
+ * and then fill in all the fields up to the --private-- marker.
+ *
+ * @ec_dev: Device to register
+ * @return 0 if ok, -ve on error
+ */
+int cros_ec_register(struct cros_ec_device *ec_dev);
+
+/**
+ * cros_ec_alloc - Allocate a new ChromeOS EC
+ *
+ * @name: Name of EC (typically the interface it connects on)
+ * @return pointer to created device, or NULL on failure
+ */
+struct cros_ec_device *cros_ec_alloc(const char *name);
+
+/**
+ * cros_ec_free - Free a ChromeOS EC, the opposite of cros_ec_alloc().
+ *
+ * @ec_dev: Device to free (call cros_ec_remove() first)
+ */
+void cros_ec_free(struct cros_ec_device *ec_dev);
+
+#endif /* __LINUX_MFD_CROS_EC_H */
--
1.8.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 2/6] mfd: Add ChromeOS EC implementation
2013-02-13 2:42 ` [PATCH v2 2/6] mfd: Add ChromeOS EC implementation Simon Glass
@ 2013-02-13 3:35 ` Joe Perches
2013-02-16 3:58 ` Simon Glass
0 siblings, 1 reply; 16+ messages in thread
From: Joe Perches @ 2013-02-13 3:35 UTC (permalink / raw)
To: Simon Glass
Cc: LKML, Samuel Ortiz, Che-Liang Chiou, Jonathan Kliegman,
Luigi Semenzato, Olof Johansson, Vincent Palatin, Grant Likely,
Rob Herring, Rob Landley, devicetree-discuss, linux-doc
On Tue, 2013-02-12 at 18:42 -0800, Simon Glass wrote:
> This is the base EC implementation, which provides a high level
> interface to the EC for use by the rest of the kernel. The actual
> communcations is dealt with by a separate protocol driver which
> registers itself with this interface.
trivial logging message comments...
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
[]
> +struct cros_ec_device *cros_ec_alloc(const char *name)
> +{
> + struct cros_ec_device *ec_dev;
> +
> + ec_dev = kzalloc(sizeof(*ec_dev), GFP_KERNEL);
> + if (ec_dev == NULL) {
> + dev_err(ec_dev->dev, "cannot allocate\n");
allocation OOM messages aren't useful as there's
a standard one on all allocs without __GFP_WARN
> +int cros_ec_register(struct cros_ec_device *ec_dev)
> +{
[]
> + ec_dev->din = kmalloc(ec_dev->din_size, GFP_KERNEL);
> + if (!ec_dev->din) {
> + err = -ENOMEM;
> + dev_err(dev, "cannot allocate din\n");
etc...
> + if (err) {
> + dev_err(dev, "failed to add mfd devices");
missing terminating newline
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 2/6] mfd: Add ChromeOS EC implementation
2013-02-13 3:35 ` Joe Perches
@ 2013-02-16 3:58 ` Simon Glass
0 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2013-02-16 3:58 UTC (permalink / raw)
To: Joe Perches
Cc: LKML, Samuel Ortiz, Che-Liang Chiou, Jonathan Kliegman,
Luigi Semenzato, Olof Johansson, Vincent Palatin, Grant Likely,
Rob Herring, Rob Landley, devicetree-discuss, linux-doc
Hi Joe,
On Tue, Feb 12, 2013 at 7:35 PM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2013-02-12 at 18:42 -0800, Simon Glass wrote:
>> This is the base EC implementation, which provides a high level
>> interface to the EC for use by the rest of the kernel. The actual
>> communcations is dealt with by a separate protocol driver which
>> registers itself with this interface.
>
> trivial logging message comments...
Thanks - I will tidy these up in the next spin.
>
>> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> []
>> +struct cros_ec_device *cros_ec_alloc(const char *name)
>> +{
>> + struct cros_ec_device *ec_dev;
>> +
>> + ec_dev = kzalloc(sizeof(*ec_dev), GFP_KERNEL);
>> + if (ec_dev == NULL) {
>> + dev_err(ec_dev->dev, "cannot allocate\n");
>
> allocation OOM messages aren't useful as there's
> a standard one on all allocs without __GFP_WARN
>
>> +int cros_ec_register(struct cros_ec_device *ec_dev)
>> +{
> []
>> + ec_dev->din = kmalloc(ec_dev->din_size, GFP_KERNEL);
>> + if (!ec_dev->din) {
>> + err = -ENOMEM;
>> + dev_err(dev, "cannot allocate din\n");
>
> etc...
>
>> + if (err) {
>> + dev_err(dev, "failed to add mfd devices");
>
> missing terminating newline
>
>
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 3/6] mfd: Add ChromeOS EC I2C driver
2013-02-13 2:42 [PATCH v2 0/6] Add ChromeOS Embedded Controller support Simon Glass
2013-02-13 2:42 ` [PATCH v2 1/6] mfd: Add ChromeOS EC messages header Simon Glass
2013-02-13 2:42 ` [PATCH v2 2/6] mfd: Add ChromeOS EC implementation Simon Glass
@ 2013-02-13 2:42 ` Simon Glass
2013-02-13 2:42 ` [PATCH v2 4/6] mfd: Add ChromeOS EC SPI driver Simon Glass
` (2 subsequent siblings)
5 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2013-02-13 2:42 UTC (permalink / raw)
To: LKML; +Cc: Samuel Ortiz, Simon Glass, Che-Liang Chiou
This uses an I2C bus to talk to the ChromeOS EC. The protocol
is defined by the EC and is fairly simple, with a length byte,
checksum, command byte and version byte (to permit easy creation
of new commands).
Signed-off-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Che-Liang Chiou <clchiou@chromium.org>
---
Changes in v2:
- Remove use of __devinit/__devexit
drivers/mfd/Kconfig | 10 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/cros_ec_i2c.c | 262 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 273 insertions(+)
create mode 100644 drivers/mfd/cros_ec_i2c.c
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 837a16b..e1cd15e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -29,6 +29,16 @@ config MFD_CROS_EC
You also ned to enable the driver for the bus you are using. The
protocol for talking to the EC is defined by the bus driver.
+config MFD_CROS_EC_I2C
+ tristate "ChromeOS Embedded Controller (I2C)"
+ depends on MFD_CROS_EC && I2C
+
+ help
+ If you say here, you get support for talking to the ChromeOS EC
+ through an I2C bus. This uses a simple byte-level protocol with
+ a checksum. Failing accesses will be retried three times to
+ improve reliability.
+
config MFD_88PM800
tristate "Support Marvell 88PM800"
depends on I2C=y && GENERIC_HARDIRQS
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 967767d..895257b 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_MFD_88PM805) += 88pm805.o 88pm80x.o
obj-$(CONFIG_MFD_SM501) += sm501.o
obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
obj-$(CONFIG_MFD_CROS_EC) += cros_ec.o
+obj-$(CONFIG_MFD_CROS_EC_I2C) += cros_ec_i2c.o
rtsx_pci-objs := rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o
obj-$(CONFIG_MFD_RTSX_PCI) += rtsx_pci.o
diff --git a/drivers/mfd/cros_ec_i2c.c b/drivers/mfd/cros_ec_i2c.c
new file mode 100644
index 0000000..fe3f2bf
--- /dev/null
+++ b/drivers/mfd/cros_ec_i2c.c
@@ -0,0 +1,262 @@
+/*
+ * ChromeOS EC multi-function device (I2C)
+ *
+ * Copyright (C) 2012 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+
+/* Since I2C can be unreliable, we retry commands */
+#define COMMAND_MAX_TRIES 3
+
+static inline struct cros_ec_device *to_ec_dev(struct device *dev)
+{
+ struct i2c_client *client = to_i2c_client(dev);
+
+ return i2c_get_clientdata(client);
+}
+
+static int cros_ec_command_xfer_noretry(struct cros_ec_device *ec_dev,
+ struct cros_ec_msg *msg)
+{
+ struct i2c_client *client = ec_dev->priv;
+ int ret = -ENOMEM;
+ int i;
+ int packet_len;
+ u8 *out_buf = NULL;
+ u8 *in_buf = NULL;
+ u8 sum;
+ struct i2c_msg i2c_msg[2];
+
+ i2c_msg[0].addr = client->addr;
+ i2c_msg[0].flags = 0;
+ i2c_msg[1].addr = client->addr;
+ i2c_msg[1].flags = I2C_M_RD;
+
+ /*
+ * allocate larger packet (one byte for checksum, one byte for
+ * length, and one for result code)
+ */
+ packet_len = msg->in_len + 3;
+ in_buf = kzalloc(packet_len, GFP_KERNEL);
+ if (!in_buf)
+ goto done;
+ i2c_msg[1].len = packet_len;
+ i2c_msg[1].buf = (char *)in_buf;
+
+ /*
+ * allocate larger packet (one byte for checksum, one for
+ * command code, one for length, and one for command version)
+ */
+ packet_len = msg->out_len + 4;
+ out_buf = kzalloc(packet_len, GFP_KERNEL);
+ if (!out_buf)
+ goto done;
+ i2c_msg[0].len = packet_len;
+ i2c_msg[0].buf = (char *)out_buf;
+
+ out_buf[0] = EC_CMD_VERSION0 + msg->version;
+ out_buf[1] = msg->cmd;
+ out_buf[2] = msg->out_len;
+
+ /* copy message payload and compute checksum */
+ sum = out_buf[0] + out_buf[1] + out_buf[2];
+ for (i = 0; i < msg->out_len; i++) {
+ out_buf[3 + i] = msg->out_buf[i];
+ sum += out_buf[3 + i];
+ }
+ out_buf[3 + msg->out_len] = sum;
+
+ /* send command to EC and read answer */
+ ret = i2c_transfer(client->adapter, i2c_msg, 2);
+ if (ret < 0) {
+ dev_err(ec_dev->dev, "i2c transfer failed: %d\n", ret);
+ goto done;
+ } else if (ret != 2) {
+ dev_err(ec_dev->dev, "failed to get response: %d\n", ret);
+ ret = -EIO;
+ goto done;
+ }
+
+ /* check response error code */
+ if (i2c_msg[1].buf[0]) {
+ dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
+ msg->cmd, i2c_msg[1].buf[0]);
+ ret = -EINVAL;
+ goto done;
+ }
+
+ /* copy response packet payload and compute checksum */
+ sum = in_buf[0] + in_buf[1];
+ for (i = 0; i < msg->in_len; i++) {
+ msg->in_buf[i] = in_buf[2 + i];
+ sum += in_buf[2 + i];
+ }
+#ifdef DEBUG
+ dev_dbg(ec_dev->dev, "packet: ");
+ for (i = 0; i < i2c_msg[1].len; i++)
+ dev_cont(ec_dev->dev, " %02x", in_buf[i]);
+ dev_cont(ec_dev->dev, ", sum = %02x\n", sum);
+#endif
+ if (sum != in_buf[2 + msg->in_len]) {
+ dev_err(ec_dev->dev, "bad packet checksum\n");
+ ret = -EBADMSG;
+ goto done;
+ }
+
+ ret = 0;
+ done:
+ kfree(in_buf);
+ kfree(out_buf);
+ return ret;
+}
+
+static int cros_ec_command_xfer(struct cros_ec_device *ec_dev,
+ struct cros_ec_msg *msg)
+{
+ int tries;
+ int ret;
+ /*
+ * Try the command a few times in case there are transmission errors.
+ * It is possible that this is overkill, but we don't completely trust
+ * i2c.
+ */
+ for (tries = 0; tries < COMMAND_MAX_TRIES; tries++) {
+ ret = cros_ec_command_xfer_noretry(ec_dev, msg);
+ if (ret >= 0)
+ return ret;
+ }
+ dev_err(ec_dev->dev, "ec_command failed with %d (%d tries)\n",
+ ret, tries);
+ return ret;
+}
+
+static const char *cros_ec_get_name(struct cros_ec_device *ec_dev)
+{
+ struct i2c_client *client = ec_dev->priv;
+
+ return client->name;
+}
+
+static const char *cros_ec_get_phys_name(struct cros_ec_device *ec_dev)
+{
+ struct i2c_client *client = ec_dev->priv;
+
+ return client->adapter->name;
+}
+
+static struct device *cros_ec_get_parent(struct cros_ec_device *ec_dev)
+{
+ struct i2c_client *client = ec_dev->priv;
+
+ return &client->dev;
+}
+
+static int cros_ec_probe_i2c(struct i2c_client *client,
+ const struct i2c_device_id *dev_id)
+{
+ struct device *dev = &client->dev;
+ struct cros_ec_device *ec_dev = NULL;
+ int err;
+
+ if (dev->of_node && !of_device_is_available(dev->of_node)) {
+ dev_warn(dev, "Device disabled by device tree\n");
+ return -ENODEV;
+ }
+
+ ec_dev = cros_ec_alloc("I2C");
+ if (!ec_dev) {
+ err = -ENOMEM;
+ dev_err(dev, "cannot create cros_ec\n");
+ goto fail;
+ }
+
+ i2c_set_clientdata(client, ec_dev);
+ ec_dev->dev = dev;
+ ec_dev->priv = client;
+ ec_dev->irq = client->irq;
+ ec_dev->command_xfer = cros_ec_command_xfer;
+ ec_dev->get_name = cros_ec_get_name;
+ ec_dev->get_phys_name = cros_ec_get_phys_name;
+ ec_dev->get_parent = cros_ec_get_parent;
+
+ err = cros_ec_register(ec_dev);
+ if (err) {
+ dev_err(dev, "cannot register EC\n");
+ goto fail_register;
+ }
+
+ return 0;
+fail_register:
+ cros_ec_free(ec_dev);
+fail:
+ return err;
+}
+
+static int cros_ec_remove_i2c(struct i2c_client *client)
+{
+ struct cros_ec_device *ec_dev = i2c_get_clientdata(client);
+
+ cros_ec_remove(ec_dev);
+ cros_ec_free(ec_dev);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int cros_ec_i2c_suspend(struct device *dev)
+{
+ struct cros_ec_device *ec_dev = to_ec_dev(dev);
+
+ return cros_ec_suspend(ec_dev);
+}
+
+static int cros_ec_i2c_resume(struct device *dev)
+{
+ struct cros_ec_device *ec_dev = to_ec_dev(dev);
+
+ return cros_ec_resume(ec_dev);
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(cros_ec_i2c_pm_ops, cros_ec_i2c_suspend,
+ cros_ec_i2c_resume);
+
+static const struct i2c_device_id cros_ec_i2c_id[] = {
+ { "cros-ec-i2c", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, cros_ec_i2c_id);
+
+static struct i2c_driver cros_ec_driver = {
+ .driver = {
+ .name = "cros-ec-i2c",
+ .owner = THIS_MODULE,
+ .pm = &cros_ec_i2c_pm_ops,
+ },
+ .probe = cros_ec_probe_i2c,
+ .remove = cros_ec_remove_i2c,
+ .id_table = cros_ec_i2c_id,
+};
+
+module_i2c_driver(cros_ec_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("ChromeOS EC multi function device");
--
1.8.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 4/6] mfd: Add ChromeOS EC SPI driver
2013-02-13 2:42 [PATCH v2 0/6] Add ChromeOS Embedded Controller support Simon Glass
` (2 preceding siblings ...)
2013-02-13 2:42 ` [PATCH v2 3/6] mfd: Add ChromeOS EC I2C driver Simon Glass
@ 2013-02-13 2:42 ` Simon Glass
2013-02-13 2:42 ` [PATCH v2 5/6] Input: matrix-keymap: Add function to read the new DT binding Simon Glass
2013-02-13 2:42 ` [PATCH v2 6/6] Input: Add ChromeOS EC keyboard driver Simon Glass
5 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2013-02-13 2:42 UTC (permalink / raw)
To: LKML; +Cc: Samuel Ortiz, Simon Glass
This uses a SPI bus to talk to the ChromeOS EC. The protocol
is defined by the EC and is fairly simple, with a length byte,
checksum, command byte and version byte (to permit easy creation
of new commands).
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Remove use of __devinit/__devexit
drivers/mfd/Kconfig | 10 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/cros_ec_spi.c | 412 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 423 insertions(+)
create mode 100644 drivers/mfd/cros_ec_spi.c
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index e1cd15e..e30b801 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -39,6 +39,16 @@ config MFD_CROS_EC_I2C
a checksum. Failing accesses will be retried three times to
improve reliability.
+config MFD_CROS_EC_SPI
+ tristate "ChromeOS Embedded Controller (SPI)"
+ depends on MFD_CROS_EC && SPI
+
+ ---help---
+ If you say Y here, you get support for talking to the ChromeOS EC
+ through a SPI bus, using a byte-level protocol. Since the EC's
+ response time cannot be guaranteed, we support ignoring
+ 'pre-amble' bytes before the response actually starts.
+
config MFD_88PM800
tristate "Support Marvell 88PM800"
depends on I2C=y && GENERIC_HARDIRQS
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 895257b..f62a583 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_MFD_SM501) += sm501.o
obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
obj-$(CONFIG_MFD_CROS_EC) += cros_ec.o
obj-$(CONFIG_MFD_CROS_EC_I2C) += cros_ec_i2c.o
+obj-$(CONFIG_MFD_CROS_EC_SPI) += cros_ec_spi.o
rtsx_pci-objs := rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o
obj-$(CONFIG_MFD_RTSX_PCI) += rtsx_pci.o
diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c
new file mode 100644
index 0000000..3a97653
--- /dev/null
+++ b/drivers/mfd/cros_ec_spi.c
@@ -0,0 +1,412 @@
+/*
+ * ChromeOS EC multi-function device (SPI)
+ *
+ * Copyright (C) 2012 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.
+ */
+
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+
+
+/* The header byte, which follows the preamble */
+#define EC_MSG_HEADER 0xec
+
+/*
+ * Number of EC preamble bytes we read at a time. Since it takes
+ * about 400-500us for the EC to respond there is not a lot of
+ * point in tuning this. If the EC could respond faster then
+ * we could increase this so that might expect the preamble and
+ * message to occur in a single transaction. However, the maximum
+ * SPI transfer size is 256 bytes, so at 5MHz we need a response
+ * time of perhaps <320us (200 bytes / 1600 bits).
+ */
+#define EC_MSG_PREAMBLE_COUNT 32
+
+/*
+ * We must get a response from the EC in 5ms. This is a very long
+ * time, but the flash write command can take 2-3ms. The EC command
+ * processing is currently not very fast (about 500us). We could
+ * look at speeding this up and making the flash write command a
+ * 'slow' command, requiring a GET_STATUS wait loop, like flash
+ * erase.
+ */
+#define EC_MSG_DEADLINE_MS 5
+
+/*
+ * Time between raising the SPI chip select (for the end of a
+ * transaction) and dropping it again (for the next transaction).
+ * If we go too fast, the EC will miss the transaction. It seems
+ * that 50us is enough with the 16MHz STM32 EC.
+ */
+#define EC_SPI_RECOVERY_TIME_NS (50 * 1000)
+
+/**
+ * struct cros_ec_spi - information about a SPI-connected EC
+ *
+ * @spi: SPI device we are connected to
+ * @last_transfer_ns: time that we last finished a transfer, or 0 if there
+ * if no record
+ */
+struct cros_ec_spi {
+ struct spi_device *spi;
+ s64 last_transfer_ns;
+};
+
+static void debug_packet(struct device *dev, const char *name, u8 *ptr,
+ int len)
+{
+#ifdef DEBUG
+ int i;
+
+ dev_dbg(dev, "%s: ", name);
+ for (i = 0; i < len; i++)
+ dev_cont(dev, " %02x", ptr[i]);
+#endif
+}
+
+/**
+ * cros_ec_spi_receive_response - Receive a response from the EC.
+ *
+ * This function has two phases: reading the preamble bytes (since if we read
+ * data from the EC before it is ready to send, we just get preamble) and
+ * reading the actual message.
+ *
+ * The received data is placed into ec_dev->din.
+ *
+ * @ec_dev: ChromeOS EC device
+ * @need_len: Number of message bytes we need to read
+ */
+static int cros_ec_spi_receive_response(struct cros_ec_device *ec_dev,
+ int need_len)
+{
+ struct cros_ec_spi *ec_spi = ec_dev->priv;
+ struct spi_transfer trans;
+ struct spi_message msg;
+ u8 *ptr, *end;
+ int ret;
+ unsigned long deadline;
+ int todo;
+
+ /* Receive data until we see the header byte */
+ deadline = jiffies + msecs_to_jiffies(EC_MSG_DEADLINE_MS);
+ do {
+ memset(&trans, '\0', sizeof(trans));
+ trans.cs_change = 1;
+ trans.rx_buf = ptr = ec_dev->din;
+ trans.len = EC_MSG_PREAMBLE_COUNT;
+
+ spi_message_init(&msg);
+ spi_message_add_tail(&trans, &msg);
+ ret = spi_sync(ec_spi->spi, &msg);
+ if (ret < 0) {
+ dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
+ return ret;
+ }
+
+ for (end = ptr + EC_MSG_PREAMBLE_COUNT; ptr != end; ptr++) {
+ if (*ptr == EC_MSG_HEADER) {
+ dev_dbg(ec_dev->dev, "msg found at %d\n",
+ ptr - ec_dev->din);
+ break;
+ }
+ }
+
+ if (time_after(jiffies, deadline)) {
+ dev_warn(ec_dev->dev, "EC failed to respond in time\n");
+ return -ETIMEDOUT;
+ }
+ } while (ptr == end);
+
+ /*
+ * ptr now points to the header byte. Copy any valid data to the
+ * start of our buffer
+ */
+ todo = end - ++ptr;
+ BUG_ON(todo < 0 || todo > ec_dev->din_size);
+ todo = min(todo, need_len);
+ memmove(ec_dev->din, ptr, todo);
+ ptr = ec_dev->din + todo;
+ dev_dbg(ec_dev->dev, "need %d, got %d bytes from preamble\n",
+ need_len, todo);
+ need_len -= todo;
+
+ /* Receive data until we have it all */
+ while (need_len > 0) {
+ /*
+ * We can't support transfers larger than the SPI FIFO size
+ * unless we have DMA. We don't have DMA on the ISP SPI ports
+ * for Exynos. We need a way of asking SPI driver for
+ * maximum-supported transfer size.
+ */
+ todo = min(need_len, 256);
+ dev_dbg(ec_dev->dev, "loop, todo=%d, need_len=%d, ptr=%d\n",
+ todo, need_len, ptr - ec_dev->din);
+
+ memset(&trans, '\0', sizeof(trans));
+ trans.cs_change = 1;
+ trans.rx_buf = ptr;
+ trans.len = todo;
+ spi_message_init(&msg);
+ spi_message_add_tail(&trans, &msg);
+
+ /* send command to EC and read answer */
+ BUG_ON((u8 *)trans.rx_buf - ec_dev->din + todo >
+ ec_dev->din_size);
+ ret = spi_sync(ec_spi->spi, &msg);
+ if (ret < 0) {
+ dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
+ return ret;
+ }
+
+ debug_packet(ec_dev->dev, "interim", ptr, todo);
+ ptr += todo;
+ need_len -= todo;
+ }
+
+ dev_dbg(ec_dev->dev, "loop done, ptr=%d\n", ptr - ec_dev->din);
+
+ return 0;
+}
+
+/**
+ * cros_ec_command_spi_xfer - Transfer a message over SPI and receive the reply
+ *
+ * @ec_dev: ChromeOS EC device
+ * @ec_msg: Message to transfer
+ */
+static int cros_ec_command_spi_xfer(struct cros_ec_device *ec_dev,
+ struct cros_ec_msg *ec_msg)
+{
+ struct cros_ec_spi *ec_spi = ec_dev->priv;
+ struct spi_transfer trans;
+ struct spi_message msg;
+ int i, len;
+ u8 *ptr;
+ int sum;
+ int ret = 0, final_ret;
+ struct timespec ts;
+
+ len = cros_ec_prepare_tx(ec_dev, ec_msg);
+ dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
+
+ /* If it's too soon to do another transaction, wait */
+ if (ec_spi->last_transfer_ns) {
+ struct timespec ts;
+ unsigned long delay; /* The delay completed so far */
+
+ ktime_get_ts(&ts);
+ delay = timespec_to_ns(&ts) - ec_spi->last_transfer_ns;
+ if (delay < EC_SPI_RECOVERY_TIME_NS)
+ ndelay(delay);
+ }
+
+ /* Transmit phase - send our message */
+ debug_packet(ec_dev->dev, "out", ec_dev->dout, len);
+ memset(&trans, '\0', sizeof(trans));
+ trans.tx_buf = ec_dev->dout;
+ trans.len = len;
+ trans.cs_change = 1;
+ spi_message_init(&msg);
+ spi_message_add_tail(&trans, &msg);
+ ret = spi_sync(ec_spi->spi, &msg);
+
+ /* Get the response */
+ if (!ret) {
+ ret = cros_ec_spi_receive_response(ec_dev,
+ ec_msg->in_len + EC_MSG_TX_PROTO_BYTES);
+ } else {
+ dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
+ }
+
+ /* turn off CS */
+ spi_message_init(&msg);
+ final_ret = spi_sync(ec_spi->spi, &msg);
+ ktime_get_ts(&ts);
+ ec_spi->last_transfer_ns = timespec_to_ns(&ts);
+ if (!ret)
+ ret = final_ret;
+ if (ret < 0) {
+ dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret);
+ return ret;
+ }
+
+ /* check response error code */
+ ptr = ec_dev->din;
+ if (ptr[0]) {
+ dev_warn(ec_dev->dev, "command 0x%02x returned an error %d\n",
+ ec_msg->cmd, ptr[0]);
+ debug_packet(ec_dev->dev, "in_err", ptr, len);
+ return -EINVAL;
+ }
+ len = ptr[1];
+ sum = ptr[0] + ptr[1];
+ if (len > ec_msg->in_len) {
+ dev_err(ec_dev->dev, "packet too long (%d bytes, expected %d)",
+ len, ec_msg->in_len);
+ return -ENOSPC;
+ }
+
+ /* copy response packet payload and compute checksum */
+ for (i = 0; i < len; i++) {
+ sum += ptr[i + 2];
+ if (ec_msg->in_len)
+ ec_msg->in_buf[i] = ptr[i + 2];
+ }
+ sum &= 0xff;
+
+ debug_packet(ec_dev->dev, "in", ptr, len + 3);
+
+ if (sum != ptr[len + 2]) {
+ dev_err(ec_dev->dev,
+ "bad packet checksum, expected %02x, got %02x\n",
+ sum, ptr[len + 2]);
+ return -EBADMSG;
+ }
+
+ return 0;
+}
+
+static const char *cros_ec_get_name(struct cros_ec_device *ec_dev)
+{
+ struct cros_ec_spi *ec_spi = ec_dev->priv;
+
+ return ec_spi->spi->modalias;
+}
+
+static const char *cros_ec_get_phys_name(struct cros_ec_device *ec_dev)
+{
+ struct cros_ec_spi *ec_spi = ec_dev->priv;
+
+ return dev_name(&ec_spi->spi->dev);
+}
+
+static struct device *cros_ec_get_parent(struct cros_ec_device *ec_dev)
+{
+ struct cros_ec_spi *ec_spi = ec_dev->priv;
+
+ return &ec_spi->spi->dev;
+}
+
+static int cros_ec_probe_spi(struct spi_device *spi)
+{
+ struct device *dev = &spi->dev;
+ struct cros_ec_device *ec_dev;
+ struct cros_ec_spi *ec_spi;
+ int err;
+
+ spi->bits_per_word = 8;
+ spi->mode = SPI_MODE_0;
+ err = spi_setup(spi);
+ if (err < 0)
+ return err;
+
+ ec_spi = kzalloc(sizeof(*ec_spi), GFP_KERNEL);
+ if (ec_spi == NULL) {
+ err = -ENOMEM;
+ dev_err(dev, "cannot allocate ec_spi\n");
+ goto fail_alloc;
+ }
+ ec_spi->spi = spi;
+ ec_dev = cros_ec_alloc("SPI");
+ if (!ec_dev) {
+ err = -ENOMEM;
+ dev_err(dev, "cannot create cros_ec\n");
+ goto fail_ec_alloc;
+ }
+ spi_set_drvdata(spi, ec_dev);
+ ec_dev->dev = dev;
+ ec_dev->priv = ec_spi;
+ ec_dev->irq = spi->irq;
+ ec_dev->command_xfer = cros_ec_command_spi_xfer;
+ ec_dev->get_name = cros_ec_get_name;
+ ec_dev->get_phys_name = cros_ec_get_phys_name;
+ ec_dev->get_parent = cros_ec_get_parent;
+ ec_dev->din_size = EC_MSG_BYTES + EC_MSG_PREAMBLE_COUNT;
+ ec_dev->dout_size = EC_MSG_BYTES;
+
+ err = cros_ec_register(ec_dev);
+ if (err) {
+ dev_err(dev, "cannot register EC\n");
+ goto fail_register;
+ }
+
+ return 0;
+
+fail_register:
+ cros_ec_free(ec_dev);
+fail_ec_alloc:
+ kfree(ec_spi);
+fail_alloc:
+ return err;
+}
+
+static int cros_ec_remove_spi(struct spi_device *spi)
+{
+ struct cros_ec_device *ec_dev;
+ struct cros_ec_spi *ec_spi;
+
+ ec_dev = spi_get_drvdata(spi);
+ ec_spi = ec_dev->priv;
+
+ cros_ec_remove(ec_dev);
+ cros_ec_free(ec_dev);
+ kfree(ec_spi);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int cros_ec_spi_suspend(struct device *dev)
+{
+ struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
+
+ return cros_ec_suspend(ec_dev);
+}
+
+static int cros_ec_spi_resume(struct device *dev)
+{
+ struct cros_ec_device *ec_dev = dev_get_drvdata(dev);
+
+ return cros_ec_resume(ec_dev);
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(cros_ec_spi_pm_ops, cros_ec_spi_suspend,
+ cros_ec_spi_resume);
+
+static const struct spi_device_id cros_ec_spi_id[] = {
+ { "cros-ec-spi", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(spi, cros_ec_spi_id);
+
+static struct spi_driver cros_ec_driver_spi = {
+ .driver = {
+ .name = "cros-ec-spi",
+ .owner = THIS_MODULE,
+ .pm = &cros_ec_spi_pm_ops,
+ },
+ .probe = cros_ec_probe_spi,
+ .remove = cros_ec_remove_spi,
+ .id_table = cros_ec_spi_id,
+};
+
+module_spi_driver(cros_ec_driver_spi);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("ChromeOS EC multi function device (SPI)");
--
1.8.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH v2 5/6] Input: matrix-keymap: Add function to read the new DT binding
2013-02-13 2:42 [PATCH v2 0/6] Add ChromeOS Embedded Controller support Simon Glass
` (3 preceding siblings ...)
2013-02-13 2:42 ` [PATCH v2 4/6] mfd: Add ChromeOS EC SPI driver Simon Glass
@ 2013-02-13 2:42 ` Simon Glass
2013-02-13 5:32 ` a0131647
2013-02-13 19:43 ` Dmitry Torokhov
2013-02-13 2:42 ` [PATCH v2 6/6] Input: Add ChromeOS EC keyboard driver Simon Glass
5 siblings, 2 replies; 16+ messages in thread
From: Simon Glass @ 2013-02-13 2:42 UTC (permalink / raw)
To: LKML
Cc: Samuel Ortiz, Simon Glass, Dmitry Torokhov, Bill Pemberton,
Mark Brown, Javier Martinez Canillas, Sourav Poddar,
Felipe Balbi, Alban Bedel, linux-input
We now have a binding which adds two parameters to the matrix keypad DT
node. This is separate from the GPIO-driven matrix keypad binding, and
unfortunately incompatible, since that uses row-gpios/col-gpios for the
row and column counts.
So the easiest option here is to provide a function for non-GPIO drivers
to use to decode the binding.
Note: We could in fact create an entirely separate structure to hold
these two fields, but it does not seem worth it, yet. If we have more
parameters then we can add this, and then refactor each driver to hold
such a structure.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Add new patch to decode matrix-keypad DT binding
drivers/input/keyboard/lpc32xx-keys.c | 11 ++++++-----
drivers/input/keyboard/omap4-keypad.c | 16 +++++-----------
drivers/input/keyboard/tca8418_keypad.c | 11 +++++++----
drivers/input/matrix-keymap.c | 20 ++++++++++++++++++++
include/linux/input/matrix_keypad.h | 11 +++++++++++
5 files changed, 49 insertions(+), 20 deletions(-)
diff --git a/drivers/input/keyboard/lpc32xx-keys.c b/drivers/input/keyboard/lpc32xx-keys.c
index 1b8add6..4218143 100644
--- a/drivers/input/keyboard/lpc32xx-keys.c
+++ b/drivers/input/keyboard/lpc32xx-keys.c
@@ -144,12 +144,13 @@ static int lpc32xx_parse_dt(struct device *dev,
{
struct device_node *np = dev->of_node;
u32 rows = 0, columns = 0;
+ int err;
- of_property_read_u32(np, "keypad,num-rows", &rows);
- of_property_read_u32(np, "keypad,num-columns", &columns);
- if (!rows || rows != columns) {
- dev_err(dev,
- "rows and columns must be specified and be equal!\n");
+ err = matrix_keypad_parse_of_params(dev, &rows, &columns);
+ if (err)
+ return err;
+ if (rows != columns) {
+ dev_err(dev, "rows and columns must be equal!\n");
return -EINVAL;
}
diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
index e25b022..1b28909 100644
--- a/drivers/input/keyboard/omap4-keypad.c
+++ b/drivers/input/keyboard/omap4-keypad.c
@@ -215,18 +215,12 @@ static int omap4_keypad_parse_dt(struct device *dev,
struct omap4_keypad *keypad_data)
{
struct device_node *np = dev->of_node;
+ int err;
- if (!np) {
- dev_err(dev, "missing DT data");
- return -EINVAL;
- }
-
- of_property_read_u32(np, "keypad,num-rows", &keypad_data->rows);
- of_property_read_u32(np, "keypad,num-columns", &keypad_data->cols);
- if (!keypad_data->rows || !keypad_data->cols) {
- dev_err(dev, "number of keypad rows/columns not specified\n");
- return -EINVAL;
- }
+ err = matrix_keypad_parse_of_params(dev, &keypad_data->rows,
+ &keypad_data->cols);
+ if (err)
+ return err;
if (of_get_property(np, "linux,input-no-autorepeat", NULL))
keypad_data->no_autorepeat = true;
diff --git a/drivers/input/keyboard/tca8418_keypad.c b/drivers/input/keyboard/tca8418_keypad.c
index a34cc67..4d5a6d5 100644
--- a/drivers/input/keyboard/tca8418_keypad.c
+++ b/drivers/input/keyboard/tca8418_keypad.c
@@ -288,17 +288,20 @@ static int tca8418_keypad_probe(struct i2c_client *client,
irq_is_gpio = pdata->irq_is_gpio;
} else {
struct device_node *np = dev->of_node;
- of_property_read_u32(np, "keypad,num-rows", &rows);
- of_property_read_u32(np, "keypad,num-columns", &cols);
+ int err;
+
+ err = matrix_keypad_parse_of_params(dev, &rows, &cols);
+ if (err)
+ return err;
rep = of_property_read_bool(np, "keypad,autorepeat");
}
- if (!rows || rows > TCA8418_MAX_ROWS) {
+ if (rows > TCA8418_MAX_ROWS) {
dev_err(dev, "invalid rows\n");
return -EINVAL;
}
- if (!cols || cols > TCA8418_MAX_COLS) {
+ if (cols > TCA8418_MAX_COLS) {
dev_err(dev, "invalid columns\n");
return -EINVAL;
}
diff --git a/drivers/input/matrix-keymap.c b/drivers/input/matrix-keymap.c
index 3ae496e..64bad81 100644
--- a/drivers/input/matrix-keymap.c
+++ b/drivers/input/matrix-keymap.c
@@ -50,6 +50,25 @@ static bool matrix_keypad_map_key(struct input_dev *input_dev,
}
#ifdef CONFIG_OF
+int matrix_keypad_parse_of_params(struct device *dev,
+ unsigned int *rows, unsigned int *cols)
+{
+ struct device_node *np = dev->of_node;
+
+ if (!np) {
+ dev_err(dev, "missing DT data");
+ return -EINVAL;
+ }
+ of_property_read_u32(np, "keypad,num-rows", rows);
+ of_property_read_u32(np, "keypad,num-columns", cols);
+ if (!*rows || !*cols) {
+ dev_err(dev, "number of keypad rows/columns not specified\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
static int matrix_keypad_parse_of_keymap(const char *propname,
unsigned int rows, unsigned int cols,
struct input_dev *input_dev)
@@ -112,6 +131,7 @@ static int matrix_keypad_parse_of_keymap(const char *propname,
* tree support is enabled).
* @rows: number of rows in target keymap array
* @cols: number of cols in target keymap array
+ * (if either/both is 0 then they will be read from the FDT if available)
* @keymap: expanded version of keymap that is suitable for use by
* matrix keyboard driver
* @input_dev: input devices for which we are setting up the keymap
diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
index 5f3aa6b..01a30cf 100644
--- a/include/linux/input/matrix_keypad.h
+++ b/include/linux/input/matrix_keypad.h
@@ -81,4 +81,15 @@ int matrix_keypad_build_keymap(const struct matrix_keymap_data *keymap_data,
unsigned short *keymap,
struct input_dev *input_dev);
+/**
+ * matrix_keypad_parse_of_params() - Read parameters from matrix-keypad node
+ *
+ * @dev: Device containing of_node
+ * @rows: Returns number of matrix rows
+ * @cols: Returns number of matrix columns
+ * @return 0 if OK, <0 on error
+ */
+int matrix_keypad_parse_of_params(struct device *dev,
+ unsigned int *rows, unsigned int *cols);
+
#endif /* _MATRIX_KEYPAD_H */
--
1.8.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 5/6] Input: matrix-keymap: Add function to read the new DT binding
2013-02-13 2:42 ` [PATCH v2 5/6] Input: matrix-keymap: Add function to read the new DT binding Simon Glass
@ 2013-02-13 5:32 ` a0131647
2013-02-13 19:43 ` Dmitry Torokhov
1 sibling, 0 replies; 16+ messages in thread
From: a0131647 @ 2013-02-13 5:32 UTC (permalink / raw)
To: Simon Glass
Cc: LKML, Samuel Ortiz, Dmitry Torokhov, Bill Pemberton, Mark Brown,
Javier Martinez Canillas, Felipe Balbi, Alban Bedel, linux-input
Hi,
On Wednesday 13 February 2013 08:12 AM, Simon Glass wrote:
> We now have a binding which adds two parameters to the matrix keypad DT
> node. This is separate from the GPIO-driven matrix keypad binding, and
> unfortunately incompatible, since that uses row-gpios/col-gpios for the
> row and column counts.
>
> So the easiest option here is to provide a function for non-GPIO drivers
> to use to decode the binding.
>
> Note: We could in fact create an entirely separate structure to hold
> these two fields, but it does not seem worth it, yet. If we have more
> parameters then we can add this, and then refactor each driver to hold
> such a structure.
>
> Signed-off-by: Simon Glass<sjg@chromium.org>
> ---
> Changes in v2:
> - Add new patch to decode matrix-keypad DT binding
>
> drivers/input/keyboard/lpc32xx-keys.c | 11 ++++++-----
> drivers/input/keyboard/omap4-keypad.c | 16 +++++-----------
> drivers/input/keyboard/tca8418_keypad.c | 11 +++++++----
> drivers/input/matrix-keymap.c | 20 ++++++++++++++++++++
> include/linux/input/matrix_keypad.h | 11 +++++++++++
> 5 files changed, 49 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/input/keyboard/lpc32xx-keys.c b/drivers/input/keyboard/lpc32xx-keys.c
> index 1b8add6..4218143 100644
> --- a/drivers/input/keyboard/lpc32xx-keys.c
> +++ b/drivers/input/keyboard/lpc32xx-keys.c
> @@ -144,12 +144,13 @@ static int lpc32xx_parse_dt(struct device *dev,
> {
> struct device_node *np = dev->of_node;
> u32 rows = 0, columns = 0;
> + int err;
>
> - of_property_read_u32(np, "keypad,num-rows",&rows);
> - of_property_read_u32(np, "keypad,num-columns",&columns);
> - if (!rows || rows != columns) {
> - dev_err(dev,
> - "rows and columns must be specified and be equal!\n");
> + err = matrix_keypad_parse_of_params(dev,&rows,&columns);
> + if (err)
> + return err;
> + if (rows != columns) {
> + dev_err(dev, "rows and columns must be equal!\n");
> return -EINVAL;
> }
>
> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
> index e25b022..1b28909 100644
> --- a/drivers/input/keyboard/omap4-keypad.c
> +++ b/drivers/input/keyboard/omap4-keypad.c
> @@ -215,18 +215,12 @@ static int omap4_keypad_parse_dt(struct device *dev,
> struct omap4_keypad *keypad_data)
> {
> struct device_node *np = dev->of_node;
> + int err;
>
> - if (!np) {
> - dev_err(dev, "missing DT data");
> - return -EINVAL;
> - }
> -
> - of_property_read_u32(np, "keypad,num-rows",&keypad_data->rows);
> - of_property_read_u32(np, "keypad,num-columns",&keypad_data->cols);
> - if (!keypad_data->rows || !keypad_data->cols) {
> - dev_err(dev, "number of keypad rows/columns not specified\n");
> - return -EINVAL;
> - }
> + err = matrix_keypad_parse_of_params(dev,&keypad_data->rows,
> + &keypad_data->cols);
> + if (err)
> + return err;
>
> if (of_get_property(np, "linux,input-no-autorepeat", NULL))
> keypad_data->no_autorepeat = true;
> diff --git a/drivers/input/keyboard/tca8418_keypad.c b/drivers/input/keyboard/tca8418_keypad.c
> index a34cc67..4d5a6d5 100644
> --- a/drivers/input/keyboard/tca8418_keypad.c
> +++ b/drivers/input/keyboard/tca8418_keypad.c
> @@ -288,17 +288,20 @@ static int tca8418_keypad_probe(struct i2c_client *client,
> irq_is_gpio = pdata->irq_is_gpio;
> } else {
> struct device_node *np = dev->of_node;
> - of_property_read_u32(np, "keypad,num-rows",&rows);
> - of_property_read_u32(np, "keypad,num-columns",&cols);
> + int err;
> +
> + err = matrix_keypad_parse_of_params(dev,&rows,&cols);
> + if (err)
> + return err;
> rep = of_property_read_bool(np, "keypad,autorepeat");
> }
>
> - if (!rows || rows> TCA8418_MAX_ROWS) {
> + if (rows> TCA8418_MAX_ROWS) {
> dev_err(dev, "invalid rows\n");
> return -EINVAL;
> }
>
> - if (!cols || cols> TCA8418_MAX_COLS) {
> + if (cols> TCA8418_MAX_COLS) {
> dev_err(dev, "invalid columns\n");
> return -EINVAL;
> }
> diff --git a/drivers/input/matrix-keymap.c b/drivers/input/matrix-keymap.c
> index 3ae496e..64bad81 100644
> --- a/drivers/input/matrix-keymap.c
> +++ b/drivers/input/matrix-keymap.c
> @@ -50,6 +50,25 @@ static bool matrix_keypad_map_key(struct input_dev *input_dev,
> }
>
> #ifdef CONFIG_OF
> +int matrix_keypad_parse_of_params(struct device *dev,
> + unsigned int *rows, unsigned int *cols)
> +{
> + struct device_node *np = dev->of_node;
> +
> + if (!np) {
> + dev_err(dev, "missing DT data");
> + return -EINVAL;
> + }
> + of_property_read_u32(np, "keypad,num-rows", rows);
> + of_property_read_u32(np, "keypad,num-columns", cols);
> + if (!*rows || !*cols) {
> + dev_err(dev, "number of keypad rows/columns not specified\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int matrix_keypad_parse_of_keymap(const char *propname,
> unsigned int rows, unsigned int cols,
> struct input_dev *input_dev)
> @@ -112,6 +131,7 @@ static int matrix_keypad_parse_of_keymap(const char *propname,
> * tree support is enabled).
> * @rows: number of rows in target keymap array
> * @cols: number of cols in target keymap array
> + * (if either/both is 0 then they will be read from the FDT if available)
> * @keymap: expanded version of keymap that is suitable for use by
> * matrix keyboard driver
> * @input_dev: input devices for which we are setting up the keymap
> diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
> index 5f3aa6b..01a30cf 100644
> --- a/include/linux/input/matrix_keypad.h
> +++ b/include/linux/input/matrix_keypad.h
> @@ -81,4 +81,15 @@ int matrix_keypad_build_keymap(const struct matrix_keymap_data *keymap_data,
> unsigned short *keymap,
> struct input_dev *input_dev);
>
> +/**
> + * matrix_keypad_parse_of_params() - Read parameters from matrix-keypad node
> + *
> + * @dev: Device containing of_node
> + * @rows: Returns number of matrix rows
> + * @cols: Returns number of matrix columns
> + * @return 0 if OK,<0 on error
> + */
> +int matrix_keypad_parse_of_params(struct device *dev,
> + unsigned int *rows, unsigned int *cols);
> +
> #endif /* _MATRIX_KEYPAD_H */
Looks good to me.You can add
Reviewed-by: Sourav Poddar <sourav.poddar@ti.com>
Also tested this patch on omap4430 sdp, keypad works fine. You can also
add a
Tested-by: Sourav Poddar <sourav.poddar@ti.com>
~Sourav
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 5/6] Input: matrix-keymap: Add function to read the new DT binding
2013-02-13 2:42 ` [PATCH v2 5/6] Input: matrix-keymap: Add function to read the new DT binding Simon Glass
2013-02-13 5:32 ` a0131647
@ 2013-02-13 19:43 ` Dmitry Torokhov
2013-02-14 5:40 ` Simon Glass
1 sibling, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2013-02-13 19:43 UTC (permalink / raw)
To: Simon Glass
Cc: LKML, Samuel Ortiz, Bill Pemberton, Mark Brown,
Javier Martinez Canillas, Sourav Poddar, Felipe Balbi,
Alban Bedel, linux-input
Hi Simon,
On Tue, Feb 12, 2013 at 06:42:25PM -0800, Simon Glass wrote:
> We now have a binding which adds two parameters to the matrix keypad DT
> node. This is separate from the GPIO-driven matrix keypad binding, and
> unfortunately incompatible, since that uses row-gpios/col-gpios for the
> row and column counts.
>
> So the easiest option here is to provide a function for non-GPIO drivers
> to use to decode the binding.
>
> Note: We could in fact create an entirely separate structure to hold
> these two fields, but it does not seem worth it, yet. If we have more
> parameters then we can add this, and then refactor each driver to hold
> such a structure.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Add new patch to decode matrix-keypad DT binding
>
> drivers/input/keyboard/lpc32xx-keys.c | 11 ++++++-----
> drivers/input/keyboard/omap4-keypad.c | 16 +++++-----------
> drivers/input/keyboard/tca8418_keypad.c | 11 +++++++----
> drivers/input/matrix-keymap.c | 20 ++++++++++++++++++++
> include/linux/input/matrix_keypad.h | 11 +++++++++++
> 5 files changed, 49 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/input/keyboard/lpc32xx-keys.c b/drivers/input/keyboard/lpc32xx-keys.c
> index 1b8add6..4218143 100644
> --- a/drivers/input/keyboard/lpc32xx-keys.c
> +++ b/drivers/input/keyboard/lpc32xx-keys.c
> @@ -144,12 +144,13 @@ static int lpc32xx_parse_dt(struct device *dev,
> {
> struct device_node *np = dev->of_node;
> u32 rows = 0, columns = 0;
> + int err;
>
> - of_property_read_u32(np, "keypad,num-rows", &rows);
> - of_property_read_u32(np, "keypad,num-columns", &columns);
> - if (!rows || rows != columns) {
> - dev_err(dev,
> - "rows and columns must be specified and be equal!\n");
> + err = matrix_keypad_parse_of_params(dev, &rows, &columns);
> + if (err)
> + return err;
> + if (rows != columns) {
> + dev_err(dev, "rows and columns must be equal!\n");
> return -EINVAL;
> }
>
> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
> index e25b022..1b28909 100644
> --- a/drivers/input/keyboard/omap4-keypad.c
> +++ b/drivers/input/keyboard/omap4-keypad.c
> @@ -215,18 +215,12 @@ static int omap4_keypad_parse_dt(struct device *dev,
> struct omap4_keypad *keypad_data)
> {
> struct device_node *np = dev->of_node;
> + int err;
>
> - if (!np) {
> - dev_err(dev, "missing DT data");
> - return -EINVAL;
> - }
> -
> - of_property_read_u32(np, "keypad,num-rows", &keypad_data->rows);
> - of_property_read_u32(np, "keypad,num-columns", &keypad_data->cols);
> - if (!keypad_data->rows || !keypad_data->cols) {
> - dev_err(dev, "number of keypad rows/columns not specified\n");
> - return -EINVAL;
> - }
> + err = matrix_keypad_parse_of_params(dev, &keypad_data->rows,
> + &keypad_data->cols);
> + if (err)
> + return err;
>
> if (of_get_property(np, "linux,input-no-autorepeat", NULL))
> keypad_data->no_autorepeat = true;
> diff --git a/drivers/input/keyboard/tca8418_keypad.c b/drivers/input/keyboard/tca8418_keypad.c
> index a34cc67..4d5a6d5 100644
> --- a/drivers/input/keyboard/tca8418_keypad.c
> +++ b/drivers/input/keyboard/tca8418_keypad.c
> @@ -288,17 +288,20 @@ static int tca8418_keypad_probe(struct i2c_client *client,
> irq_is_gpio = pdata->irq_is_gpio;
> } else {
> struct device_node *np = dev->of_node;
> - of_property_read_u32(np, "keypad,num-rows", &rows);
> - of_property_read_u32(np, "keypad,num-columns", &cols);
> + int err;
> +
> + err = matrix_keypad_parse_of_params(dev, &rows, &cols);
> + if (err)
> + return err;
> rep = of_property_read_bool(np, "keypad,autorepeat");
> }
>
> - if (!rows || rows > TCA8418_MAX_ROWS) {
> + if (rows > TCA8418_MAX_ROWS) {
Why? Your new function only checks rows != 0 for the DT case, but we
also need to validate platform data supplied values.
> dev_err(dev, "invalid rows\n");
> return -EINVAL;
> }
>
> - if (!cols || cols > TCA8418_MAX_COLS) {
> + if (cols > TCA8418_MAX_COLS) {
> dev_err(dev, "invalid columns\n");
> return -EINVAL;
> }
> diff --git a/drivers/input/matrix-keymap.c b/drivers/input/matrix-keymap.c
> index 3ae496e..64bad81 100644
> --- a/drivers/input/matrix-keymap.c
> +++ b/drivers/input/matrix-keymap.c
> @@ -50,6 +50,25 @@ static bool matrix_keypad_map_key(struct input_dev *input_dev,
> }
>
> #ifdef CONFIG_OF
> +int matrix_keypad_parse_of_params(struct device *dev,
> + unsigned int *rows, unsigned int *cols)
> +{
> + struct device_node *np = dev->of_node;
> +
> + if (!np) {
> + dev_err(dev, "missing DT data");
> + return -EINVAL;
> + }
> + of_property_read_u32(np, "keypad,num-rows", rows);
> + of_property_read_u32(np, "keypad,num-columns", cols);
> + if (!*rows || !*cols) {
> + dev_err(dev, "number of keypad rows/columns not specified\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int matrix_keypad_parse_of_keymap(const char *propname,
> unsigned int rows, unsigned int cols,
> struct input_dev *input_dev)
> @@ -112,6 +131,7 @@ static int matrix_keypad_parse_of_keymap(const char *propname,
> * tree support is enabled).
> * @rows: number of rows in target keymap array
> * @cols: number of cols in target keymap array
> + * (if either/both is 0 then they will be read from the FDT if available)
Huh?
> * @keymap: expanded version of keymap that is suitable for use by
> * matrix keyboard driver
> * @input_dev: input devices for which we are setting up the keymap
> diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
> index 5f3aa6b..01a30cf 100644
> --- a/include/linux/input/matrix_keypad.h
> +++ b/include/linux/input/matrix_keypad.h
> @@ -81,4 +81,15 @@ int matrix_keypad_build_keymap(const struct matrix_keymap_data *keymap_data,
> unsigned short *keymap,
> struct input_dev *input_dev);
>
> +/**
> + * matrix_keypad_parse_of_params() - Read parameters from matrix-keypad node
> + *
> + * @dev: Device containing of_node
> + * @rows: Returns number of matrix rows
> + * @cols: Returns number of matrix columns
> + * @return 0 if OK, <0 on error
> + */
> +int matrix_keypad_parse_of_params(struct device *dev,
> + unsigned int *rows, unsigned int *cols);
> +
The new function needs a non-CONFIG_OF stub as some drivers
(tca8418-keypad for example) use it in common code.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 5/6] Input: matrix-keymap: Add function to read the new DT binding
2013-02-13 19:43 ` Dmitry Torokhov
@ 2013-02-14 5:40 ` Simon Glass
0 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2013-02-14 5:40 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: LKML, Samuel Ortiz, Bill Pemberton, Mark Brown,
Javier Martinez Canillas, Sourav Poddar, Felipe Balbi,
Alban Bedel, linux-input
Hi Dmitry,
On Wed, Feb 13, 2013 at 11:43 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Simon,
>
> On Tue, Feb 12, 2013 at 06:42:25PM -0800, Simon Glass wrote:
>> We now have a binding which adds two parameters to the matrix keypad DT
>> node. This is separate from the GPIO-driven matrix keypad binding, and
>> unfortunately incompatible, since that uses row-gpios/col-gpios for the
>> row and column counts.
>>
>> So the easiest option here is to provide a function for non-GPIO drivers
>> to use to decode the binding.
>>
>> Note: We could in fact create an entirely separate structure to hold
>> these two fields, but it does not seem worth it, yet. If we have more
>> parameters then we can add this, and then refactor each driver to hold
>> such a structure.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> Changes in v2:
>> - Add new patch to decode matrix-keypad DT binding
>>
>> drivers/input/keyboard/lpc32xx-keys.c | 11 ++++++-----
>> drivers/input/keyboard/omap4-keypad.c | 16 +++++-----------
>> drivers/input/keyboard/tca8418_keypad.c | 11 +++++++----
>> drivers/input/matrix-keymap.c | 20 ++++++++++++++++++++
>> include/linux/input/matrix_keypad.h | 11 +++++++++++
>> 5 files changed, 49 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/input/keyboard/lpc32xx-keys.c b/drivers/input/keyboard/lpc32xx-keys.c
>> index 1b8add6..4218143 100644
>> --- a/drivers/input/keyboard/lpc32xx-keys.c
>> +++ b/drivers/input/keyboard/lpc32xx-keys.c
>> @@ -144,12 +144,13 @@ static int lpc32xx_parse_dt(struct device *dev,
>> {
>> struct device_node *np = dev->of_node;
>> u32 rows = 0, columns = 0;
>> + int err;
>>
>> - of_property_read_u32(np, "keypad,num-rows", &rows);
>> - of_property_read_u32(np, "keypad,num-columns", &columns);
>> - if (!rows || rows != columns) {
>> - dev_err(dev,
>> - "rows and columns must be specified and be equal!\n");
>> + err = matrix_keypad_parse_of_params(dev, &rows, &columns);
>> + if (err)
>> + return err;
>> + if (rows != columns) {
>> + dev_err(dev, "rows and columns must be equal!\n");
>> return -EINVAL;
>> }
>>
>> diff --git a/drivers/input/keyboard/omap4-keypad.c b/drivers/input/keyboard/omap4-keypad.c
>> index e25b022..1b28909 100644
>> --- a/drivers/input/keyboard/omap4-keypad.c
>> +++ b/drivers/input/keyboard/omap4-keypad.c
>> @@ -215,18 +215,12 @@ static int omap4_keypad_parse_dt(struct device *dev,
>> struct omap4_keypad *keypad_data)
>> {
>> struct device_node *np = dev->of_node;
>> + int err;
>>
>> - if (!np) {
>> - dev_err(dev, "missing DT data");
>> - return -EINVAL;
>> - }
>> -
>> - of_property_read_u32(np, "keypad,num-rows", &keypad_data->rows);
>> - of_property_read_u32(np, "keypad,num-columns", &keypad_data->cols);
>> - if (!keypad_data->rows || !keypad_data->cols) {
>> - dev_err(dev, "number of keypad rows/columns not specified\n");
>> - return -EINVAL;
>> - }
>> + err = matrix_keypad_parse_of_params(dev, &keypad_data->rows,
>> + &keypad_data->cols);
>> + if (err)
>> + return err;
>>
>> if (of_get_property(np, "linux,input-no-autorepeat", NULL))
>> keypad_data->no_autorepeat = true;
>> diff --git a/drivers/input/keyboard/tca8418_keypad.c b/drivers/input/keyboard/tca8418_keypad.c
>> index a34cc67..4d5a6d5 100644
>> --- a/drivers/input/keyboard/tca8418_keypad.c
>> +++ b/drivers/input/keyboard/tca8418_keypad.c
>> @@ -288,17 +288,20 @@ static int tca8418_keypad_probe(struct i2c_client *client,
>> irq_is_gpio = pdata->irq_is_gpio;
>> } else {
>> struct device_node *np = dev->of_node;
>> - of_property_read_u32(np, "keypad,num-rows", &rows);
>> - of_property_read_u32(np, "keypad,num-columns", &cols);
>> + int err;
>> +
>> + err = matrix_keypad_parse_of_params(dev, &rows, &cols);
>> + if (err)
>> + return err;
>> rep = of_property_read_bool(np, "keypad,autorepeat");
>> }
>>
>> - if (!rows || rows > TCA8418_MAX_ROWS) {
>> + if (rows > TCA8418_MAX_ROWS) {
>
> Why? Your new function only checks rows != 0 for the DT case, but we
> also need to validate platform data supplied values.
Thanks for reviewing.
OK - will fix. It seems odd that that driver calls device tree code
even when CONFIG_OF is not defined.
>
>> dev_err(dev, "invalid rows\n");
>> return -EINVAL;
>> }
>>
>> - if (!cols || cols > TCA8418_MAX_COLS) {
>> + if (cols > TCA8418_MAX_COLS) {
>> dev_err(dev, "invalid columns\n");
>> return -EINVAL;
>> }
>> diff --git a/drivers/input/matrix-keymap.c b/drivers/input/matrix-keymap.c
>> index 3ae496e..64bad81 100644
>> --- a/drivers/input/matrix-keymap.c
>> +++ b/drivers/input/matrix-keymap.c
>> @@ -50,6 +50,25 @@ static bool matrix_keypad_map_key(struct input_dev *input_dev,
>> }
>>
>> #ifdef CONFIG_OF
>> +int matrix_keypad_parse_of_params(struct device *dev,
>> + unsigned int *rows, unsigned int *cols)
>> +{
>> + struct device_node *np = dev->of_node;
>> +
>> + if (!np) {
>> + dev_err(dev, "missing DT data");
>> + return -EINVAL;
>> + }
>> + of_property_read_u32(np, "keypad,num-rows", rows);
>> + of_property_read_u32(np, "keypad,num-columns", cols);
>> + if (!*rows || !*cols) {
>> + dev_err(dev, "number of keypad rows/columns not specified\n");
>> + return -EINVAL;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int matrix_keypad_parse_of_keymap(const char *propname,
>> unsigned int rows, unsigned int cols,
>> struct input_dev *input_dev)
>> @@ -112,6 +131,7 @@ static int matrix_keypad_parse_of_keymap(const char *propname,
>> * tree support is enabled).
>> * @rows: number of rows in target keymap array
>> * @cols: number of cols in target keymap array
>> + * (if either/both is 0 then they will be read from the FDT if available)
>
> Huh?
Not sure what happened there, some sort of stuff-up, but that code
isn't there now.
>
>> * @keymap: expanded version of keymap that is suitable for use by
>> * matrix keyboard driver
>> * @input_dev: input devices for which we are setting up the keymap
>> diff --git a/include/linux/input/matrix_keypad.h b/include/linux/input/matrix_keypad.h
>> index 5f3aa6b..01a30cf 100644
>> --- a/include/linux/input/matrix_keypad.h
>> +++ b/include/linux/input/matrix_keypad.h
>> @@ -81,4 +81,15 @@ int matrix_keypad_build_keymap(const struct matrix_keymap_data *keymap_data,
>> unsigned short *keymap,
>> struct input_dev *input_dev);
>>
>> +/**
>> + * matrix_keypad_parse_of_params() - Read parameters from matrix-keypad node
>> + *
>> + * @dev: Device containing of_node
>> + * @rows: Returns number of matrix rows
>> + * @cols: Returns number of matrix columns
>> + * @return 0 if OK, <0 on error
>> + */
>> +int matrix_keypad_parse_of_params(struct device *dev,
>> + unsigned int *rows, unsigned int *cols);
>> +
>
> The new function needs a non-CONFIG_OF stub as some drivers
> (tca8418-keypad for example) use it in common code.
OK, not sure whether I am permitted to put a static inline in the
header, but will go with that for now.
>
> Thanks.
>
> --
> Dmitry
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 6/6] Input: Add ChromeOS EC keyboard driver
2013-02-13 2:42 [PATCH v2 0/6] Add ChromeOS Embedded Controller support Simon Glass
` (4 preceding siblings ...)
2013-02-13 2:42 ` [PATCH v2 5/6] Input: matrix-keymap: Add function to read the new DT binding Simon Glass
@ 2013-02-13 2:42 ` Simon Glass
2013-02-13 20:02 ` Dmitry Torokhov
5 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2013-02-13 2:42 UTC (permalink / raw)
To: LKML
Cc: Samuel Ortiz, Simon Glass, Luigi Semenzato, Vincent Palatin,
Grant Likely, Rob Herring, Rob Landley, Dmitry Torokhov,
Felipe Balbi, Sourav Poddar, Tony Lindgren, Greg Kroah-Hartman,
Mike A. Chan, Jun Nakajima, Tom Keel, devicetree-discuss,
linux-doc, linux-input
Use the key-matrix layer to interpret key scan information from the EC
and inject input based on the FDT-supplied key map. This driver registers
itself with the ChromeOS EC driver to perform communications.
Additional FDT bindings are provided to specify rows/columns and the
auto-repeat information.
Signed-off-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Luigi Semenzato <semenzato@chromium.org>
Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
---
Changes in v2:
- Remove use of __devinit/__devexit
- Use function to read matrix-keypad parameters from DT
- Remove key autorepeat parameters from DT binding and driver
- Use unsigned int for rows/cols
.../devicetree/bindings/input/cros-ec-keyb.txt | 72 ++++
drivers/input/keyboard/Kconfig | 12 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/cros_ec_keyb.c | 394 +++++++++++++++++++++
4 files changed, 479 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/cros-ec-keyb.txt
create mode 100644 drivers/input/keyboard/cros_ec_keyb.c
diff --git a/Documentation/devicetree/bindings/input/cros-ec-keyb.txt b/Documentation/devicetree/bindings/input/cros-ec-keyb.txt
new file mode 100644
index 0000000..0f6355c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cros-ec-keyb.txt
@@ -0,0 +1,72 @@
+ChromeOS EC Keyboard
+
+Google's ChromeOS EC Keyboard is a simple matrix keyboard implemented on
+a separate EC (Embedded Controller) device. It provides a message for reading
+key scans from the EC. These are then converted into keycodes for processing
+by the kernel.
+
+This binding is based on matrix-keymap.txt and extends/modifies it as follows:
+
+Required properties:
+- compatible: "google,cros-ec-keyb"
+
+Optional properties:
+- google,needs-ghost-filter: True to enable a ghost filter for the matrix
+keyboard. This is recommended if the EC does not have its own logic or
+hardware for this.
+
+
+Example:
+
+cros-ec-keyb {
+ compatible = "google,cros-ec-keyb";
+ keypad,num-rows = <8>;
+ keypad,num-columns = <13>;
+ google,needs-ghost-filter;
+ /*
+ * Keymap entries take the form of 0xRRCCKKKK where
+ * RR=Row CC=Column KKKK=Key Code
+ * The values below are for a US keyboard layout and
+ * are taken from the Linux driver. Note that the
+ * 102ND key is not used for US keyboards.
+ */
+ linux,keymap = <
+ /* CAPSLCK F1 B F10 */
+ 0x0001003a 0x0002003b 0x00030030 0x00040044
+ /* N = R_ALT ESC */
+ 0x00060031 0x0008000d 0x000a0064 0x01010001
+ /* F4 G F7 H */
+ 0x0102003e 0x01030022 0x01040041 0x01060023
+ /* ' F9 BKSPACE L_CTRL */
+ 0x01080028 0x01090043 0x010b000e 0x0200001d
+ /* TAB F3 T F6 */
+ 0x0201000f 0x0202003d 0x02030014 0x02040040
+ /* ] Y 102ND [ */
+ 0x0205001b 0x02060015 0x02070056 0x0208001a
+ /* F8 GRAVE F2 5 */
+ 0x02090042 0x03010029 0x0302003c 0x03030006
+ /* F5 6 - \ */
+ 0x0304003f 0x03060007 0x0308000c 0x030b002b
+ /* R_CTRL A D F */
+ 0x04000061 0x0401001e 0x04020020 0x04030021
+ /* S K J ; */
+ 0x0404001f 0x04050025 0x04060024 0x04080027
+ /* L ENTER Z C */
+ 0x04090026 0x040b001c 0x0501002c 0x0502002e
+ /* V X , M */
+ 0x0503002f 0x0504002d 0x05050033 0x05060032
+ /* L_SHIFT / . SPACE */
+ 0x0507002a 0x05080035 0x05090034 0x050B0039
+ /* 1 3 4 2 */
+ 0x06010002 0x06020004 0x06030005 0x06040003
+ /* 8 7 0 9 */
+ 0x06050009 0x06060008 0x0608000b 0x0609000a
+ /* L_ALT DOWN RIGHT Q */
+ 0x060a0038 0x060b006c 0x060c006a 0x07010010
+ /* E R W I */
+ 0x07020012 0x07030013 0x07040011 0x07050017
+ /* U R_SHIFT P O */
+ 0x07060016 0x07070036 0x07080019 0x07090018
+ /* UP LEFT */
+ 0x070b0067 0x070c0069>;
+};
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 078305e..3a70be7 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -628,4 +628,16 @@ config KEYBOARD_W90P910
To compile this driver as a module, choose M here: the
module will be called w90p910_keypad.
+config KEYBOARD_CROS_EC
+ tristate "ChromeOS EC keyboard"
+ select INPUT_MATRIXKMAP
+ select MFD_CROS_EC
+ help
+ Say Y here to enable the matrix keyboard used by ChromeOS devices
+ and implemented on the ChromeOS EC. You must enable one bus option
+ (MFD_CROS_EC_I2C or MFD_CROS_EC_SPI) to use this.
+
+ To compile this driver as a module, choose M here: the
+ module will be called cros_ec_keyb.
+
endif
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 49b1645..0c43e8c 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_KEYBOARD_AMIGA) += amikbd.o
obj-$(CONFIG_KEYBOARD_ATARI) += atakbd.o
obj-$(CONFIG_KEYBOARD_ATKBD) += atkbd.o
obj-$(CONFIG_KEYBOARD_BFIN) += bf54x-keys.o
+obj-$(CONFIG_KEYBOARD_CROS_EC) += cros_ec_keyb.o
obj-$(CONFIG_KEYBOARD_DAVINCI) += davinci_keyscan.o
obj-$(CONFIG_KEYBOARD_EP93XX) += ep93xx_keypad.o
obj-$(CONFIG_KEYBOARD_GOLDFISH_EVENTS) += goldfish_events.o
diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
new file mode 100644
index 0000000..43e5be2
--- /dev/null
+++ b/drivers/input/keyboard/cros_ec_keyb.c
@@ -0,0 +1,394 @@
+/*
+ * ChromeOS EC keyboard driver
+ *
+ * Copyright (C) 2012 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/module.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/kernel.h>
+#include <linux/notifier.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/input/matrix_keypad.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+
+/*
+ * @rows: Number of rows in the keypad
+ * @cols: Number of columns in the keypad
+ * @row_shift: log2 or number of rows, rounded up
+ * @keymap_data: Matrix keymap data used to convert to keyscan values
+ * @ghost_filter: true to enable the matrix key-ghosting filter
+ * @old_state: Previous state of the keyboard matrix (used to calc deltas)
+ * @dev: Device pointer
+ * @idev: Input device
+ * @ec: Top level ChromeOS device to use to talk to EC
+ * @event_notifier: interrupt event notifier for transport devices
+ * @wake_notifier: wake notfier for client devices (e.g. keyboard). This
+ * indicates to sub-drivers that we have woken up from resume but we
+ * were not a wakeup source.
+ */
+struct cros_ec_keyb {
+ unsigned int rows;
+ unsigned int cols;
+ int row_shift;
+ const struct matrix_keymap_data *keymap_data;
+ bool ghost_filter;
+ /*
+ * old_state[matrix code] is 1 when the most recent (valid)
+ * communication with the keyboard indicated that the key at row/col
+ * was in the pressed state.
+ */
+ uint8_t *old_state;
+
+ struct device *dev;
+ struct input_dev *idev;
+ struct cros_ec_device *ec;
+ struct notifier_block notifier;
+ struct notifier_block wake_notifier;
+};
+
+
+/*
+ * Sends a single key event to the input layer.
+ */
+static inline void cros_ec_keyb_send_key_event(struct cros_ec_keyb *ckdev,
+ int row, int col, int pressed)
+{
+ struct input_dev *idev = ckdev->idev;
+ int code = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
+ const unsigned short *keycodes = idev->keycode;
+
+ input_report_key(idev, keycodes[code], pressed);
+}
+
+/*
+ * Returns true when there is at least one combination of pressed keys that
+ * results in ghosting.
+ */
+static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
+{
+ int col, row;
+ int mask;
+ int pressed_in_row[ckdev->rows];
+ int row_has_teeth[ckdev->rows];
+
+ memset(pressed_in_row, '\0', sizeof(pressed_in_row));
+ memset(row_has_teeth, '\0', sizeof(row_has_teeth));
+ /*
+ * Ghosting happens if for any pressed key X there are other keys
+ * pressed both in the same row and column of X as, for instance,
+ * in the following diagram:
+ *
+ * . . Y . g .
+ * . . . . . .
+ * . . . . . .
+ * . . X . Z .
+ *
+ * In this case only X, Y, and Z are pressed, but g appears to be
+ * pressed too (see Wikipedia).
+ *
+ * We can detect ghosting in a single pass (*) over the keyboard state
+ * by maintaining two arrays. pressed_in_row counts how many pressed
+ * keys we have found in a row. row_has_teeth is true if any of the
+ * pressed keys for this row has other pressed keys in its column. If
+ * at any point of the scan we find that a row has multiple pressed
+ * keys, and at least one of them is at the intersection with a column
+ * with multiple pressed keys, we're sure there is ghosting.
+ * Conversely, if there is ghosting, we will detect such situation for
+ * at least one key during the pass.
+ *
+ * (*) This looks linear in the number of keys, but it's not. We can
+ * cheat because the number of rows is small.
+ */
+ for (row = 0; row < ckdev->rows; row++) {
+ mask = 1 << row;
+ for (col = 0; col < ckdev->cols; col++) {
+ if (buf[col] & mask) {
+ pressed_in_row[row] += 1;
+ row_has_teeth[row] |= buf[col] & ~mask;
+ if (pressed_in_row[row] > 1 &&
+ row_has_teeth[row]) {
+ /* ghosting */
+ dev_dbg(ckdev->dev,
+ "ghost found at: r%d c%d,"
+ " pressed %d, teeth 0x%x\n",
+ row, col, pressed_in_row[row],
+ row_has_teeth[row]);
+ return true;
+ }
+ }
+ }
+ }
+ return false;
+}
+
+/*
+ * Compares the new keyboard state to the old one and produces key
+ * press/release events accordingly. The keyboard state is 13 bytes (one byte
+ * per column)
+ */
+static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
+ uint8_t *kb_state, int len)
+{
+ int col, row;
+ int new_state;
+ int num_cols;
+
+ num_cols = len;
+
+ if (ckdev->ghost_filter && cros_ec_keyb_has_ghosting(ckdev, kb_state)) {
+ /*
+ * Simple-minded solution: ignore this state. The obvious
+ * improvement is to only ignore changes to keys involved in
+ * the ghosting, but process the other changes.
+ */
+ dev_dbg(ckdev->dev, "ghosting found\n");
+ return;
+ }
+
+ for (col = 0; col < ckdev->cols; col++) {
+ for (row = 0; row < ckdev->rows; row++) {
+ int code = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
+
+ new_state = kb_state[col] & (1 << row);
+ if (!!new_state != ckdev->old_state[code]) {
+ dev_dbg(ckdev->dev,
+ "changed: [r%d c%d]: byte %02x\n",
+ row, col, new_state);
+ }
+ if (new_state && !ckdev->old_state[code]) {
+ /* key press */
+ cros_ec_keyb_send_key_event(ckdev, row, col, 1);
+ ckdev->old_state[code] = 1;
+ } else if (!new_state && ckdev->old_state[code]) {
+ /* key release */
+ cros_ec_keyb_send_key_event(ckdev, row, col, 0);
+ ckdev->old_state[code] = 0;
+ }
+ }
+ }
+ input_sync(ckdev->idev);
+}
+
+static int cros_ec_keyb_open(struct input_dev *dev)
+{
+ struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
+ int ret;
+
+ ret = blocking_notifier_chain_register(&ckdev->ec->event_notifier,
+ &ckdev->notifier);
+ if (ret)
+ return ret;
+ ret = blocking_notifier_chain_register(&ckdev->ec->wake_notifier,
+ &ckdev->wake_notifier);
+ if (ret) {
+ blocking_notifier_chain_unregister(
+ &ckdev->ec->event_notifier, &ckdev->notifier);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void cros_ec_keyb_close(struct input_dev *dev)
+{
+ struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
+
+ blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
+ &ckdev->notifier);
+ blocking_notifier_chain_unregister(&ckdev->ec->wake_notifier,
+ &ckdev->wake_notifier);
+}
+
+static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
+{
+ return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
+ kb_state, ckdev->cols);
+}
+
+static int cros_ec_keyb_work(struct notifier_block *nb,
+ unsigned long state, void *_notify)
+{
+ int ret;
+ struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
+ notifier);
+ uint8_t kb_state[ckdev->cols];
+
+ ret = cros_ec_keyb_get_state(ckdev, kb_state);
+ if (ret >= 0)
+ cros_ec_keyb_process(ckdev, kb_state, ret);
+
+ return NOTIFY_DONE;
+}
+
+/* On resume, clear any keys in the buffer */
+static int cros_ec_keyb_clear_keyboard(struct notifier_block *nb,
+ unsigned long state, void *_notify)
+{
+ struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
+ wake_notifier);
+ uint8_t old_state[ckdev->cols];
+ uint8_t new_state[ckdev->cols];
+ unsigned long duration;
+ int i, ret;
+
+ /*
+ * Keep reading until we see that the scan state does not change.
+ * That indicates that we are done.
+ *
+ * Assume that the EC keyscan buffer is at most 32 deep.
+ */
+ duration = jiffies;
+ ret = cros_ec_keyb_get_state(ckdev, new_state);
+ for (i = 1; !ret && i < 32; i++) {
+ memcpy(old_state, new_state, sizeof(old_state));
+ ret = cros_ec_keyb_get_state(ckdev, new_state);
+ if (0 == memcmp(old_state, new_state, sizeof(old_state)))
+ break;
+ }
+ duration = jiffies - duration;
+ dev_info(ckdev->dev, "Discarded %d keyscan(s) in %dus\n", i,
+ jiffies_to_usecs(duration));
+
+ return 0;
+}
+
+static const struct of_device_id cros_ec_kbc_of_match[] = {
+ { .compatible = "google,cros-ec-keyb", },
+ { },
+};
+
+static int cros_ec_keyb_probe(struct platform_device *pdev)
+{
+ struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
+ struct device *dev = ec->dev;
+ struct cros_ec_keyb *ckdev = NULL;
+ struct input_dev *idev = NULL;
+ struct device_node *np;
+ int err;
+
+ np = of_find_matching_node(NULL, cros_ec_kbc_of_match);
+
+ ckdev = kzalloc(sizeof(*ckdev), GFP_KERNEL);
+ if (!ckdev) {
+ dev_err(dev, "cannot allocate memory for ckdev\n");
+ return -ENOMEM;
+ }
+ pdev->dev.of_node = np;
+ err = matrix_keypad_parse_of_params(&pdev->dev, &ckdev->rows,
+ &ckdev->cols);
+ if (err)
+ goto fail_alloc_dev;
+
+ idev = input_allocate_device();
+ if (!idev) {
+ err = -ENOMEM;
+ dev_err(dev, "cannot allocate memory for input device\n");
+ goto fail_alloc_dev;
+ }
+
+ ckdev->ec = ec;
+ ckdev->notifier.notifier_call = cros_ec_keyb_work;
+ ckdev->wake_notifier.notifier_call = cros_ec_keyb_clear_keyboard;
+ ckdev->dev = dev;
+ dev_set_drvdata(&pdev->dev, ckdev);
+
+ idev->name = ec->get_name(ec);
+ idev->phys = ec->get_phys_name(ec);
+ __set_bit(EV_REP, idev->evbit);
+
+ idev->id.bustype = BUS_VIRTUAL;
+ idev->id.version = 1;
+ idev->id.product = 0;
+ idev->dev.parent = &pdev->dev;
+ idev->open = cros_ec_keyb_open;
+ idev->close = cros_ec_keyb_close;
+
+ ckdev->ghost_filter = of_property_read_bool(np,
+ "google,needs-ghost-filter");
+
+ err = matrix_keypad_build_keymap(NULL, NULL, ckdev->rows, ckdev->cols,
+ NULL, idev);
+ if (err) {
+ dev_err(dev, "cannot build key matrix\n");
+ goto fail_matrix;
+ }
+
+ ckdev->row_shift = get_count_order(ckdev->cols);
+ ckdev->old_state = kzalloc(idev->keycodemax, GFP_KERNEL);
+ if (!ckdev->old_state) {
+ dev_err(dev, "Cannot allocate memory for old_state\n");
+ err = -ENOMEM;
+ goto fail_old_state;
+ }
+
+ input_set_capability(idev, EV_MSC, MSC_SCAN);
+ input_set_drvdata(idev, ckdev);
+ ckdev->idev = idev;
+ err = input_register_device(ckdev->idev);
+ if (err) {
+ dev_err(dev, "cannot register input device\n");
+ goto fail_register;
+ }
+
+ return 0;
+
+fail_register:
+ kfree(ckdev->old_state);
+fail_old_state:
+ kfree(idev->keycode);
+fail_matrix:
+ input_free_device(idev);
+fail_alloc_dev:
+ kfree(ckdev);
+ return err;
+}
+
+static int cros_ec_keyb_remove(struct platform_device *pdev)
+{
+ struct cros_ec_keyb *ckdev = dev_get_drvdata(&pdev->dev);
+ struct input_dev *idev = ckdev->idev;
+
+ /* I believe we leak a matrix_keymap here */
+ input_unregister_device(idev);
+ kfree(ckdev->old_state);
+ kfree(idev->keycode);
+ input_free_device(idev);
+ kfree(ckdev);
+
+ return 0;
+}
+
+static struct platform_driver cros_ec_keyb_driver = {
+ .probe = cros_ec_keyb_probe,
+ .remove = cros_ec_keyb_remove,
+ .driver = {
+ .name = "cros-ec-keyb",
+ },
+};
+
+module_platform_driver(cros_ec_keyb_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("ChromeOS EC keyboard driver");
+MODULE_ALIAS("platform:cros-ec-keyb");
--
1.8.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH v2 6/6] Input: Add ChromeOS EC keyboard driver
2013-02-13 2:42 ` [PATCH v2 6/6] Input: Add ChromeOS EC keyboard driver Simon Glass
@ 2013-02-13 20:02 ` Dmitry Torokhov
2013-02-14 6:45 ` Simon Glass
0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2013-02-13 20:02 UTC (permalink / raw)
To: Simon Glass
Cc: LKML, Samuel Ortiz, Luigi Semenzato, Vincent Palatin,
Grant Likely, Rob Herring, Rob Landley, Felipe Balbi,
Sourav Poddar, Tony Lindgren, Greg Kroah-Hartman, Mike A. Chan,
Jun Nakajima, Tom Keel, devicetree-discuss, linux-doc,
linux-input
Hi SImon,
On Tue, Feb 12, 2013 at 06:42:26PM -0800, Simon Glass wrote:
> Use the key-matrix layer to interpret key scan information from the EC
> and inject input based on the FDT-supplied key map. This driver registers
> itself with the ChromeOS EC driver to perform communications.
>
> Additional FDT bindings are provided to specify rows/columns and the
> auto-repeat information.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Luigi Semenzato <semenzato@chromium.org>
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> ---
> Changes in v2:
> - Remove use of __devinit/__devexit
> - Use function to read matrix-keypad parameters from DT
> - Remove key autorepeat parameters from DT binding and driver
> - Use unsigned int for rows/cols
>
> .../devicetree/bindings/input/cros-ec-keyb.txt | 72 ++++
> drivers/input/keyboard/Kconfig | 12 +
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/cros_ec_keyb.c | 394 +++++++++++++++++++++
> 4 files changed, 479 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/cros-ec-keyb.txt
> create mode 100644 drivers/input/keyboard/cros_ec_keyb.c
>
> diff --git a/Documentation/devicetree/bindings/input/cros-ec-keyb.txt b/Documentation/devicetree/bindings/input/cros-ec-keyb.txt
> new file mode 100644
> index 0000000..0f6355c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/cros-ec-keyb.txt
> @@ -0,0 +1,72 @@
> +ChromeOS EC Keyboard
> +
> +Google's ChromeOS EC Keyboard is a simple matrix keyboard implemented on
> +a separate EC (Embedded Controller) device. It provides a message for reading
> +key scans from the EC. These are then converted into keycodes for processing
> +by the kernel.
> +
> +This binding is based on matrix-keymap.txt and extends/modifies it as follows:
> +
> +Required properties:
> +- compatible: "google,cros-ec-keyb"
> +
> +Optional properties:
> +- google,needs-ghost-filter: True to enable a ghost filter for the matrix
> +keyboard. This is recommended if the EC does not have its own logic or
> +hardware for this.
> +
> +
> +Example:
> +
> +cros-ec-keyb {
> + compatible = "google,cros-ec-keyb";
> + keypad,num-rows = <8>;
> + keypad,num-columns = <13>;
> + google,needs-ghost-filter;
> + /*
> + * Keymap entries take the form of 0xRRCCKKKK where
> + * RR=Row CC=Column KKKK=Key Code
> + * The values below are for a US keyboard layout and
> + * are taken from the Linux driver. Note that the
> + * 102ND key is not used for US keyboards.
> + */
> + linux,keymap = <
> + /* CAPSLCK F1 B F10 */
> + 0x0001003a 0x0002003b 0x00030030 0x00040044
> + /* N = R_ALT ESC */
> + 0x00060031 0x0008000d 0x000a0064 0x01010001
> + /* F4 G F7 H */
> + 0x0102003e 0x01030022 0x01040041 0x01060023
> + /* ' F9 BKSPACE L_CTRL */
> + 0x01080028 0x01090043 0x010b000e 0x0200001d
> + /* TAB F3 T F6 */
> + 0x0201000f 0x0202003d 0x02030014 0x02040040
> + /* ] Y 102ND [ */
> + 0x0205001b 0x02060015 0x02070056 0x0208001a
> + /* F8 GRAVE F2 5 */
> + 0x02090042 0x03010029 0x0302003c 0x03030006
> + /* F5 6 - \ */
> + 0x0304003f 0x03060007 0x0308000c 0x030b002b
> + /* R_CTRL A D F */
> + 0x04000061 0x0401001e 0x04020020 0x04030021
> + /* S K J ; */
> + 0x0404001f 0x04050025 0x04060024 0x04080027
> + /* L ENTER Z C */
> + 0x04090026 0x040b001c 0x0501002c 0x0502002e
> + /* V X , M */
> + 0x0503002f 0x0504002d 0x05050033 0x05060032
> + /* L_SHIFT / . SPACE */
> + 0x0507002a 0x05080035 0x05090034 0x050B0039
> + /* 1 3 4 2 */
> + 0x06010002 0x06020004 0x06030005 0x06040003
> + /* 8 7 0 9 */
> + 0x06050009 0x06060008 0x0608000b 0x0609000a
> + /* L_ALT DOWN RIGHT Q */
> + 0x060a0038 0x060b006c 0x060c006a 0x07010010
> + /* E R W I */
> + 0x07020012 0x07030013 0x07040011 0x07050017
> + /* U R_SHIFT P O */
> + 0x07060016 0x07070036 0x07080019 0x07090018
> + /* UP LEFT */
> + 0x070b0067 0x070c0069>;
> +};
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 078305e..3a70be7 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -628,4 +628,16 @@ config KEYBOARD_W90P910
> To compile this driver as a module, choose M here: the
> module will be called w90p910_keypad.
>
> +config KEYBOARD_CROS_EC
> + tristate "ChromeOS EC keyboard"
> + select INPUT_MATRIXKMAP
> + select MFD_CROS_EC
Is this select safe? I.e. does MFD_CROS_EC depend on anything else?
> + help
> + Say Y here to enable the matrix keyboard used by ChromeOS devices
> + and implemented on the ChromeOS EC. You must enable one bus option
> + (MFD_CROS_EC_I2C or MFD_CROS_EC_SPI) to use this.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called cros_ec_keyb.
> +
> endif
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index 49b1645..0c43e8c 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -11,6 +11,7 @@ obj-$(CONFIG_KEYBOARD_AMIGA) += amikbd.o
> obj-$(CONFIG_KEYBOARD_ATARI) += atakbd.o
> obj-$(CONFIG_KEYBOARD_ATKBD) += atkbd.o
> obj-$(CONFIG_KEYBOARD_BFIN) += bf54x-keys.o
> +obj-$(CONFIG_KEYBOARD_CROS_EC) += cros_ec_keyb.o
> obj-$(CONFIG_KEYBOARD_DAVINCI) += davinci_keyscan.o
> obj-$(CONFIG_KEYBOARD_EP93XX) += ep93xx_keypad.o
> obj-$(CONFIG_KEYBOARD_GOLDFISH_EVENTS) += goldfish_events.o
> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
> new file mode 100644
> index 0000000..43e5be2
> --- /dev/null
> +++ b/drivers/input/keyboard/cros_ec_keyb.c
> @@ -0,0 +1,394 @@
> +/*
> + * ChromeOS EC keyboard driver
> + *
> + * Copyright (C) 2012 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/module.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
> +#include <linux/kernel.h>
> +#include <linux/notifier.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/input/matrix_keypad.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +
> +/*
> + * @rows: Number of rows in the keypad
> + * @cols: Number of columns in the keypad
> + * @row_shift: log2 or number of rows, rounded up
> + * @keymap_data: Matrix keymap data used to convert to keyscan values
> + * @ghost_filter: true to enable the matrix key-ghosting filter
> + * @old_state: Previous state of the keyboard matrix (used to calc deltas)
> + * @dev: Device pointer
> + * @idev: Input device
> + * @ec: Top level ChromeOS device to use to talk to EC
> + * @event_notifier: interrupt event notifier for transport devices
> + * @wake_notifier: wake notfier for client devices (e.g. keyboard). This
> + * indicates to sub-drivers that we have woken up from resume but we
> + * were not a wakeup source.
> + */
> +struct cros_ec_keyb {
> + unsigned int rows;
> + unsigned int cols;
> + int row_shift;
> + const struct matrix_keymap_data *keymap_data;
> + bool ghost_filter;
> + /*
> + * old_state[matrix code] is 1 when the most recent (valid)
> + * communication with the keyboard indicated that the key at row/col
> + * was in the pressed state.
> + */
> + uint8_t *old_state;
> +
> + struct device *dev;
> + struct input_dev *idev;
> + struct cros_ec_device *ec;
> + struct notifier_block notifier;
> + struct notifier_block wake_notifier;
> +};
> +
> +
> +/*
> + * Sends a single key event to the input layer.
> + */
> +static inline void cros_ec_keyb_send_key_event(struct cros_ec_keyb *ckdev,
> + int row, int col, int pressed)
> +{
> + struct input_dev *idev = ckdev->idev;
> + int code = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
> + const unsigned short *keycodes = idev->keycode;
> +
> + input_report_key(idev, keycodes[code], pressed);
> +}
> +
> +/*
> + * Returns true when there is at least one combination of pressed keys that
> + * results in ghosting.
> + */
> +static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
> +{
> + int col, row;
> + int mask;
> + int pressed_in_row[ckdev->rows];
> + int row_has_teeth[ckdev->rows];
> +
> + memset(pressed_in_row, '\0', sizeof(pressed_in_row));
> + memset(row_has_teeth, '\0', sizeof(row_has_teeth));
> + /*
> + * Ghosting happens if for any pressed key X there are other keys
> + * pressed both in the same row and column of X as, for instance,
> + * in the following diagram:
> + *
> + * . . Y . g .
> + * . . . . . .
> + * . . . . . .
> + * . . X . Z .
> + *
> + * In this case only X, Y, and Z are pressed, but g appears to be
> + * pressed too (see Wikipedia).
> + *
> + * We can detect ghosting in a single pass (*) over the keyboard state
> + * by maintaining two arrays. pressed_in_row counts how many pressed
> + * keys we have found in a row. row_has_teeth is true if any of the
> + * pressed keys for this row has other pressed keys in its column. If
> + * at any point of the scan we find that a row has multiple pressed
> + * keys, and at least one of them is at the intersection with a column
> + * with multiple pressed keys, we're sure there is ghosting.
> + * Conversely, if there is ghosting, we will detect such situation for
> + * at least one key during the pass.
> + *
> + * (*) This looks linear in the number of keys, but it's not. We can
> + * cheat because the number of rows is small.
> + */
> + for (row = 0; row < ckdev->rows; row++) {
> + mask = 1 << row;
> + for (col = 0; col < ckdev->cols; col++) {
> + if (buf[col] & mask) {
> + pressed_in_row[row] += 1;
Just ++ please.
> + row_has_teeth[row] |= buf[col] & ~mask;
> + if (pressed_in_row[row] > 1 &&
> + row_has_teeth[row]) {
> + /* ghosting */
> + dev_dbg(ckdev->dev,
> + "ghost found at: r%d c%d,"
> + " pressed %d, teeth 0x%x\n",
Please do not break message strings even if they push you over 80 columns.
> + row, col, pressed_in_row[row],
> + row_has_teeth[row]);
> + return true;
> + }
I am confused why you need pressed_in_row and row_has_teeth arrays as
you are working with one row at a time.
Also, can we move inner loop into a separate function?
> + }
> + }
> + }
> + return false;
> +}
> +
> +/*
> + * Compares the new keyboard state to the old one and produces key
> + * press/release events accordingly. The keyboard state is 13 bytes (one byte
> + * per column)
> + */
> +static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
> + uint8_t *kb_state, int len)
> +{
> + int col, row;
> + int new_state;
> + int num_cols;
> +
> + num_cols = len;
> +
> + if (ckdev->ghost_filter && cros_ec_keyb_has_ghosting(ckdev, kb_state)) {
> + /*
> + * Simple-minded solution: ignore this state. The obvious
> + * improvement is to only ignore changes to keys involved in
> + * the ghosting, but process the other changes.
> + */
> + dev_dbg(ckdev->dev, "ghosting found\n");
> + return;
> + }
> +
> + for (col = 0; col < ckdev->cols; col++) {
> + for (row = 0; row < ckdev->rows; row++) {
> + int code = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
> +
> + new_state = kb_state[col] & (1 << row);
> + if (!!new_state != ckdev->old_state[code]) {
> + dev_dbg(ckdev->dev,
> + "changed: [r%d c%d]: byte %02x\n",
> + row, col, new_state);
> + }
> + if (new_state && !ckdev->old_state[code]) {
> + /* key press */
> + cros_ec_keyb_send_key_event(ckdev, row, col, 1);
> + ckdev->old_state[code] = 1;
> + } else if (!new_state && ckdev->old_state[code]) {
> + /* key release */
> + cros_ec_keyb_send_key_event(ckdev, row, col, 0);
> + ckdev->old_state[code] = 0;
> + }
Should not all of the above be:
if (!!new_state != test_bit(code, dev->key)) {
dev_dbg(ckdev->dev,
"changed: [r%d c%d]: byte %02x\n",
row, col, new_state);
input_report_key(idev, keycodes[code], new_state);
}
and yo can get rid of old_state altogether?
> + }
> + }
> + input_sync(ckdev->idev);
> +}
> +
> +static int cros_ec_keyb_open(struct input_dev *dev)
> +{
> + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> + int ret;
> +
> + ret = blocking_notifier_chain_register(&ckdev->ec->event_notifier,
> + &ckdev->notifier);
> + if (ret)
> + return ret;
> + ret = blocking_notifier_chain_register(&ckdev->ec->wake_notifier,
> + &ckdev->wake_notifier);
> + if (ret) {
> + blocking_notifier_chain_unregister(
> + &ckdev->ec->event_notifier, &ckdev->notifier);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static void cros_ec_keyb_close(struct input_dev *dev)
> +{
> + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> +
> + blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
> + &ckdev->notifier);
> + blocking_notifier_chain_unregister(&ckdev->ec->wake_notifier,
> + &ckdev->wake_notifier);
Why is this done via a notifier instead of regular resume method?
> +}
> +
> +static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
> +{
> + return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
> + kb_state, ckdev->cols);
> +}
> +
> +static int cros_ec_keyb_work(struct notifier_block *nb,
> + unsigned long state, void *_notify)
> +{
> + int ret;
> + struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> + notifier);
> + uint8_t kb_state[ckdev->cols];
> +
> + ret = cros_ec_keyb_get_state(ckdev, kb_state);
> + if (ret >= 0)
> + cros_ec_keyb_process(ckdev, kb_state, ret);
> +
> + return NOTIFY_DONE;
> +}
> +
> +/* On resume, clear any keys in the buffer */
> +static int cros_ec_keyb_clear_keyboard(struct notifier_block *nb,
> + unsigned long state, void *_notify)
> +{
> + struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
> + wake_notifier);
> + uint8_t old_state[ckdev->cols];
> + uint8_t new_state[ckdev->cols];
> + unsigned long duration;
> + int i, ret;
> +
> + /*
> + * Keep reading until we see that the scan state does not change.
> + * That indicates that we are done.
> + *
> + * Assume that the EC keyscan buffer is at most 32 deep.
> + */
> + duration = jiffies;
> + ret = cros_ec_keyb_get_state(ckdev, new_state);
> + for (i = 1; !ret && i < 32; i++) {
> + memcpy(old_state, new_state, sizeof(old_state));
> + ret = cros_ec_keyb_get_state(ckdev, new_state);
> + if (0 == memcmp(old_state, new_state, sizeof(old_state)))
> + break;
> + }
> + duration = jiffies - duration;
> + dev_info(ckdev->dev, "Discarded %d keyscan(s) in %dus\n", i,
> + jiffies_to_usecs(duration));
> +
> + return 0;
> +}
> +
> +static const struct of_device_id cros_ec_kbc_of_match[] = {
> + { .compatible = "google,cros-ec-keyb", },
> + { },
> +};
> +
> +static int cros_ec_keyb_probe(struct platform_device *pdev)
> +{
> + struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
> + struct device *dev = ec->dev;
> + struct cros_ec_keyb *ckdev = NULL;
> + struct input_dev *idev = NULL;
> + struct device_node *np;
> + int err;
> +
> + np = of_find_matching_node(NULL, cros_ec_kbc_of_match);
And if we don't find it?
> +
> + ckdev = kzalloc(sizeof(*ckdev), GFP_KERNEL);
> + if (!ckdev) {
> + dev_err(dev, "cannot allocate memory for ckdev\n");
> + return -ENOMEM;
> + }
> + pdev->dev.of_node = np;
Huh? I'd expect the platform device be fully set up (including DT data)
before the driver is called.
> + err = matrix_keypad_parse_of_params(&pdev->dev, &ckdev->rows,
> + &ckdev->cols);
> + if (err)
> + goto fail_alloc_dev;
> +
> + idev = input_allocate_device();
> + if (!idev) {
> + err = -ENOMEM;
> + dev_err(dev, "cannot allocate memory for input device\n");
> + goto fail_alloc_dev;
> + }
> +
> + ckdev->ec = ec;
> + ckdev->notifier.notifier_call = cros_ec_keyb_work;
> + ckdev->wake_notifier.notifier_call = cros_ec_keyb_clear_keyboard;
> + ckdev->dev = dev;
> + dev_set_drvdata(&pdev->dev, ckdev);
> +
> + idev->name = ec->get_name(ec);
> + idev->phys = ec->get_phys_name(ec);
> + __set_bit(EV_REP, idev->evbit);
> +
> + idev->id.bustype = BUS_VIRTUAL;
> + idev->id.version = 1;
> + idev->id.product = 0;
> + idev->dev.parent = &pdev->dev;
> + idev->open = cros_ec_keyb_open;
> + idev->close = cros_ec_keyb_close;
> +
> + ckdev->ghost_filter = of_property_read_bool(np,
> + "google,needs-ghost-filter");
> +
> + err = matrix_keypad_build_keymap(NULL, NULL, ckdev->rows, ckdev->cols,
> + NULL, idev);
> + if (err) {
> + dev_err(dev, "cannot build key matrix\n");
> + goto fail_matrix;
> + }
> +
> + ckdev->row_shift = get_count_order(ckdev->cols);
> + ckdev->old_state = kzalloc(idev->keycodemax, GFP_KERNEL);
> + if (!ckdev->old_state) {
> + dev_err(dev, "Cannot allocate memory for old_state\n");
> + err = -ENOMEM;
> + goto fail_old_state;
> + }
Not needed I believe.
> +
> + input_set_capability(idev, EV_MSC, MSC_SCAN);
> + input_set_drvdata(idev, ckdev);
> + ckdev->idev = idev;
> + err = input_register_device(ckdev->idev);
> + if (err) {
> + dev_err(dev, "cannot register input device\n");
> + goto fail_register;
> + }
> +
> + return 0;
> +
> +fail_register:
> + kfree(ckdev->old_state);
> +fail_old_state:
> + kfree(idev->keycode);
> +fail_matrix:
> + input_free_device(idev);
> +fail_alloc_dev:
> + kfree(ckdev);
> + return err;
> +}
> +
> +static int cros_ec_keyb_remove(struct platform_device *pdev)
> +{
> + struct cros_ec_keyb *ckdev = dev_get_drvdata(&pdev->dev);
platform_get_drvdata() please.
> + struct input_dev *idev = ckdev->idev;
> +
> + /* I believe we leak a matrix_keymap here */
How? It is devm-managed.
> + input_unregister_device(idev);
> + kfree(ckdev->old_state);
> + kfree(idev->keycode);
And since it is devm-managed you should not free it yourself. Actually
idev is most likely gone at this point already.
> + input_free_device(idev);
Do not call input_free_device() after input_unregister_device().
> + kfree(ckdev);
> +
> + return 0;
> +}
> +
> +static struct platform_driver cros_ec_keyb_driver = {
> + .probe = cros_ec_keyb_probe,
> + .remove = cros_ec_keyb_remove,
> + .driver = {
> + .name = "cros-ec-keyb",
> + },
> +};
> +
> +module_platform_driver(cros_ec_keyb_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("ChromeOS EC keyboard driver");
> +MODULE_ALIAS("platform:cros-ec-keyb");
> --
> 1.8.1
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 6/6] Input: Add ChromeOS EC keyboard driver
2013-02-13 20:02 ` Dmitry Torokhov
@ 2013-02-14 6:45 ` Simon Glass
2013-02-14 17:31 ` Dmitry Torokhov
0 siblings, 1 reply; 16+ messages in thread
From: Simon Glass @ 2013-02-14 6:45 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: LKML, Samuel Ortiz, Luigi Semenzato, Vincent Palatin,
Grant Likely, Rob Herring, Rob Landley, Felipe Balbi,
Sourav Poddar, Tony Lindgren, Greg Kroah-Hartman, Mike A. Chan,
Jun Nakajima, Tom Keel, devicetree-discuss, linux-doc,
linux-input
Hi Dmitry,
On Wed, Feb 13, 2013 at 12:02 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi SImon,
>
> On Tue, Feb 12, 2013 at 06:42:26PM -0800, Simon Glass wrote:
>> Use the key-matrix layer to interpret key scan information from the EC
>> and inject input based on the FDT-supplied key map. This driver registers
>> itself with the ChromeOS EC driver to perform communications.
>>
>> Additional FDT bindings are provided to specify rows/columns and the
>> auto-repeat information.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> Signed-off-by: Luigi Semenzato <semenzato@chromium.org>
>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>> ---
>> Changes in v2:
>> - Remove use of __devinit/__devexit
>> - Use function to read matrix-keypad parameters from DT
>> - Remove key autorepeat parameters from DT binding and driver
>> - Use unsigned int for rows/cols
Thanks for all the review comments.
>>
>> .../devicetree/bindings/input/cros-ec-keyb.txt | 72 ++++
>> drivers/input/keyboard/Kconfig | 12 +
>> drivers/input/keyboard/Makefile | 1 +
>> drivers/input/keyboard/cros_ec_keyb.c | 394 +++++++++++++++++++++
>> 4 files changed, 479 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/input/cros-ec-keyb.txt
>> create mode 100644 drivers/input/keyboard/cros_ec_keyb.c
>>
>> diff --git a/Documentation/devicetree/bindings/input/cros-ec-keyb.txt b/Documentation/devicetree/bindings/input/cros-ec-keyb.txt
>> new file mode 100644
>> index 0000000..0f6355c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/cros-ec-keyb.txt
>> @@ -0,0 +1,72 @@
>> +ChromeOS EC Keyboard
>> +
>> +Google's ChromeOS EC Keyboard is a simple matrix keyboard implemented on
>> +a separate EC (Embedded Controller) device. It provides a message for reading
>> +key scans from the EC. These are then converted into keycodes for processing
>> +by the kernel.
>> +
>> +This binding is based on matrix-keymap.txt and extends/modifies it as follows:
>> +
>> +Required properties:
>> +- compatible: "google,cros-ec-keyb"
>> +
>> +Optional properties:
>> +- google,needs-ghost-filter: True to enable a ghost filter for the matrix
>> +keyboard. This is recommended if the EC does not have its own logic or
>> +hardware for this.
>> +
>> +
>> +Example:
>> +
>> +cros-ec-keyb {
>> + compatible = "google,cros-ec-keyb";
>> + keypad,num-rows = <8>;
>> + keypad,num-columns = <13>;
>> + google,needs-ghost-filter;
>> + /*
>> + * Keymap entries take the form of 0xRRCCKKKK where
>> + * RR=Row CC=Column KKKK=Key Code
>> + * The values below are for a US keyboard layout and
>> + * are taken from the Linux driver. Note that the
>> + * 102ND key is not used for US keyboards.
>> + */
>> + linux,keymap = <
>> + /* CAPSLCK F1 B F10 */
>> + 0x0001003a 0x0002003b 0x00030030 0x00040044
>> + /* N = R_ALT ESC */
>> + 0x00060031 0x0008000d 0x000a0064 0x01010001
>> + /* F4 G F7 H */
>> + 0x0102003e 0x01030022 0x01040041 0x01060023
>> + /* ' F9 BKSPACE L_CTRL */
>> + 0x01080028 0x01090043 0x010b000e 0x0200001d
>> + /* TAB F3 T F6 */
>> + 0x0201000f 0x0202003d 0x02030014 0x02040040
>> + /* ] Y 102ND [ */
>> + 0x0205001b 0x02060015 0x02070056 0x0208001a
>> + /* F8 GRAVE F2 5 */
>> + 0x02090042 0x03010029 0x0302003c 0x03030006
>> + /* F5 6 - \ */
>> + 0x0304003f 0x03060007 0x0308000c 0x030b002b
>> + /* R_CTRL A D F */
>> + 0x04000061 0x0401001e 0x04020020 0x04030021
>> + /* S K J ; */
>> + 0x0404001f 0x04050025 0x04060024 0x04080027
>> + /* L ENTER Z C */
>> + 0x04090026 0x040b001c 0x0501002c 0x0502002e
>> + /* V X , M */
>> + 0x0503002f 0x0504002d 0x05050033 0x05060032
>> + /* L_SHIFT / . SPACE */
>> + 0x0507002a 0x05080035 0x05090034 0x050B0039
>> + /* 1 3 4 2 */
>> + 0x06010002 0x06020004 0x06030005 0x06040003
>> + /* 8 7 0 9 */
>> + 0x06050009 0x06060008 0x0608000b 0x0609000a
>> + /* L_ALT DOWN RIGHT Q */
>> + 0x060a0038 0x060b006c 0x060c006a 0x07010010
>> + /* E R W I */
>> + 0x07020012 0x07030013 0x07040011 0x07050017
>> + /* U R_SHIFT P O */
>> + 0x07060016 0x07070036 0x07080019 0x07090018
>> + /* UP LEFT */
>> + 0x070b0067 0x070c0069>;
>> +};
>> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
>> index 078305e..3a70be7 100644
>> --- a/drivers/input/keyboard/Kconfig
>> +++ b/drivers/input/keyboard/Kconfig
>> @@ -628,4 +628,16 @@ config KEYBOARD_W90P910
>> To compile this driver as a module, choose M here: the
>> module will be called w90p910_keypad.
>>
>> +config KEYBOARD_CROS_EC
>> + tristate "ChromeOS EC keyboard"
>> + select INPUT_MATRIXKMAP
>> + select MFD_CROS_EC
>
> Is this select safe? I.e. does MFD_CROS_EC depend on anything else?
I'll remove it, since it isn't required, and it's true that it does
need other things.
>
>> + help
>> + Say Y here to enable the matrix keyboard used by ChromeOS devices
>> + and implemented on the ChromeOS EC. You must enable one bus option
>> + (MFD_CROS_EC_I2C or MFD_CROS_EC_SPI) to use this.
>> +
>> + To compile this driver as a module, choose M here: the
>> + module will be called cros_ec_keyb.
>> +
>> endif
>> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
>> index 49b1645..0c43e8c 100644
>> --- a/drivers/input/keyboard/Makefile
>> +++ b/drivers/input/keyboard/Makefile
>> @@ -11,6 +11,7 @@ obj-$(CONFIG_KEYBOARD_AMIGA) += amikbd.o
>> obj-$(CONFIG_KEYBOARD_ATARI) += atakbd.o
>> obj-$(CONFIG_KEYBOARD_ATKBD) += atkbd.o
>> obj-$(CONFIG_KEYBOARD_BFIN) += bf54x-keys.o
>> +obj-$(CONFIG_KEYBOARD_CROS_EC) += cros_ec_keyb.o
>> obj-$(CONFIG_KEYBOARD_DAVINCI) += davinci_keyscan.o
>> obj-$(CONFIG_KEYBOARD_EP93XX) += ep93xx_keypad.o
>> obj-$(CONFIG_KEYBOARD_GOLDFISH_EVENTS) += goldfish_events.o
>> diff --git a/drivers/input/keyboard/cros_ec_keyb.c b/drivers/input/keyboard/cros_ec_keyb.c
>> new file mode 100644
>> index 0000000..43e5be2
>> --- /dev/null
>> +++ b/drivers/input/keyboard/cros_ec_keyb.c
>> @@ -0,0 +1,394 @@
>> +/*
>> + * ChromeOS EC keyboard driver
>> + *
>> + * Copyright (C) 2012 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/module.h>
>> +#include <linux/i2c.h>
>> +#include <linux/input.h>
>> +#include <linux/kernel.h>
>> +#include <linux/notifier.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/input/matrix_keypad.h>
>> +#include <linux/mfd/cros_ec.h>
>> +#include <linux/mfd/cros_ec_commands.h>
>> +
>> +/*
>> + * @rows: Number of rows in the keypad
>> + * @cols: Number of columns in the keypad
>> + * @row_shift: log2 or number of rows, rounded up
>> + * @keymap_data: Matrix keymap data used to convert to keyscan values
>> + * @ghost_filter: true to enable the matrix key-ghosting filter
>> + * @old_state: Previous state of the keyboard matrix (used to calc deltas)
>> + * @dev: Device pointer
>> + * @idev: Input device
>> + * @ec: Top level ChromeOS device to use to talk to EC
>> + * @event_notifier: interrupt event notifier for transport devices
>> + * @wake_notifier: wake notfier for client devices (e.g. keyboard). This
>> + * indicates to sub-drivers that we have woken up from resume but we
>> + * were not a wakeup source.
>> + */
>> +struct cros_ec_keyb {
>> + unsigned int rows;
>> + unsigned int cols;
>> + int row_shift;
>> + const struct matrix_keymap_data *keymap_data;
>> + bool ghost_filter;
>> + /*
>> + * old_state[matrix code] is 1 when the most recent (valid)
>> + * communication with the keyboard indicated that the key at row/col
>> + * was in the pressed state.
>> + */
>> + uint8_t *old_state;
>> +
>> + struct device *dev;
>> + struct input_dev *idev;
>> + struct cros_ec_device *ec;
>> + struct notifier_block notifier;
>> + struct notifier_block wake_notifier;
>> +};
>> +
>> +
>> +/*
>> + * Sends a single key event to the input layer.
>> + */
>> +static inline void cros_ec_keyb_send_key_event(struct cros_ec_keyb *ckdev,
>> + int row, int col, int pressed)
>> +{
>> + struct input_dev *idev = ckdev->idev;
>> + int code = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
>> + const unsigned short *keycodes = idev->keycode;
>> +
>> + input_report_key(idev, keycodes[code], pressed);
>> +}
>> +
>> +/*
>> + * Returns true when there is at least one combination of pressed keys that
>> + * results in ghosting.
>> + */
>> +static bool cros_ec_keyb_has_ghosting(struct cros_ec_keyb *ckdev, uint8_t *buf)
>> +{
>> + int col, row;
>> + int mask;
>> + int pressed_in_row[ckdev->rows];
>> + int row_has_teeth[ckdev->rows];
>> +
>> + memset(pressed_in_row, '\0', sizeof(pressed_in_row));
>> + memset(row_has_teeth, '\0', sizeof(row_has_teeth));
>> + /*
>> + * Ghosting happens if for any pressed key X there are other keys
>> + * pressed both in the same row and column of X as, for instance,
>> + * in the following diagram:
>> + *
>> + * . . Y . g .
>> + * . . . . . .
>> + * . . . . . .
>> + * . . X . Z .
>> + *
>> + * In this case only X, Y, and Z are pressed, but g appears to be
>> + * pressed too (see Wikipedia).
>> + *
>> + * We can detect ghosting in a single pass (*) over the keyboard state
>> + * by maintaining two arrays. pressed_in_row counts how many pressed
>> + * keys we have found in a row. row_has_teeth is true if any of the
>> + * pressed keys for this row has other pressed keys in its column. If
>> + * at any point of the scan we find that a row has multiple pressed
>> + * keys, and at least one of them is at the intersection with a column
>> + * with multiple pressed keys, we're sure there is ghosting.
>> + * Conversely, if there is ghosting, we will detect such situation for
>> + * at least one key during the pass.
>> + *
>> + * (*) This looks linear in the number of keys, but it's not. We can
>> + * cheat because the number of rows is small.
>> + */
>> + for (row = 0; row < ckdev->rows; row++) {
>> + mask = 1 << row;
>> + for (col = 0; col < ckdev->cols; col++) {
>> + if (buf[col] & mask) {
>> + pressed_in_row[row] += 1;
>
> Just ++ please.
Done
>
>> + row_has_teeth[row] |= buf[col] & ~mask;
>> + if (pressed_in_row[row] > 1 &&
>> + row_has_teeth[row]) {
>> + /* ghosting */
>> + dev_dbg(ckdev->dev,
>> + "ghost found at: r%d c%d,"
>> + " pressed %d, teeth 0x%x\n",
>
> Please do not break message strings even if they push you over 80 columns.
Done
>
>> + row, col, pressed_in_row[row],
>> + row_has_teeth[row]);
>> + return true;
>> + }
>
> I am confused why you need pressed_in_row and row_has_teeth arrays as
> you are working with one row at a time.
Hmmm I never did quite grok that code. I can't see why we need an
array, so have changed it.
>
> Also, can we move inner loop into a separate function?
Done
>
>> + }
>> + }
>> + }
>> + return false;
>> +}
>> +
>> +/*
>> + * Compares the new keyboard state to the old one and produces key
>> + * press/release events accordingly. The keyboard state is 13 bytes (one byte
>> + * per column)
>> + */
>> +static void cros_ec_keyb_process(struct cros_ec_keyb *ckdev,
>> + uint8_t *kb_state, int len)
>> +{
>> + int col, row;
>> + int new_state;
>> + int num_cols;
>> +
>> + num_cols = len;
>> +
>> + if (ckdev->ghost_filter && cros_ec_keyb_has_ghosting(ckdev, kb_state)) {
>> + /*
>> + * Simple-minded solution: ignore this state. The obvious
>> + * improvement is to only ignore changes to keys involved in
>> + * the ghosting, but process the other changes.
>> + */
>> + dev_dbg(ckdev->dev, "ghosting found\n");
>> + return;
>> + }
>> +
>> + for (col = 0; col < ckdev->cols; col++) {
>> + for (row = 0; row < ckdev->rows; row++) {
>> + int code = MATRIX_SCAN_CODE(row, col, ckdev->row_shift);
>> +
>> + new_state = kb_state[col] & (1 << row);
>> + if (!!new_state != ckdev->old_state[code]) {
>> + dev_dbg(ckdev->dev,
>> + "changed: [r%d c%d]: byte %02x\n",
>> + row, col, new_state);
>> + }
>> + if (new_state && !ckdev->old_state[code]) {
>> + /* key press */
>> + cros_ec_keyb_send_key_event(ckdev, row, col, 1);
>> + ckdev->old_state[code] = 1;
>> + } else if (!new_state && ckdev->old_state[code]) {
>> + /* key release */
>> + cros_ec_keyb_send_key_event(ckdev, row, col, 0);
>> + ckdev->old_state[code] = 0;
>> + }
>
> Should not all of the above be:
>
> if (!!new_state != test_bit(code, dev->key)) {
> dev_dbg(ckdev->dev,
> "changed: [r%d c%d]: byte %02x\n",
> row, col, new_state);
>
> input_report_key(idev, keycodes[code], new_state);
> }
>
> and yo can get rid of old_state altogether?
That's cool. Done.
>
>> + }
>> + }
>> + input_sync(ckdev->idev);
>> +}
>> +
>> +static int cros_ec_keyb_open(struct input_dev *dev)
>> +{
>> + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
>> + int ret;
>> +
>> + ret = blocking_notifier_chain_register(&ckdev->ec->event_notifier,
>> + &ckdev->notifier);
>> + if (ret)
>> + return ret;
>> + ret = blocking_notifier_chain_register(&ckdev->ec->wake_notifier,
>> + &ckdev->wake_notifier);
>> + if (ret) {
>> + blocking_notifier_chain_unregister(
>> + &ckdev->ec->event_notifier, &ckdev->notifier);
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void cros_ec_keyb_close(struct input_dev *dev)
>> +{
>> + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
>> +
>> + blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
>> + &ckdev->notifier);
>> + blocking_notifier_chain_unregister(&ckdev->ec->wake_notifier,
>> + &ckdev->wake_notifier);
>
> Why is this done via a notifier instead of regular resume method?
Because we only call the notifer in resume when we were not waking on
a keyboard event. We use it to flush the keyboard. It was a late
change so there might be a better way, but this driver does not have a
resume handler.
>
>> +}
>> +
>> +static int cros_ec_keyb_get_state(struct cros_ec_keyb *ckdev, uint8_t *kb_state)
>> +{
>> + return ckdev->ec->command_recv(ckdev->ec, EC_CMD_MKBP_STATE,
>> + kb_state, ckdev->cols);
>> +}
>> +
>> +static int cros_ec_keyb_work(struct notifier_block *nb,
>> + unsigned long state, void *_notify)
>> +{
>> + int ret;
>> + struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
>> + notifier);
>> + uint8_t kb_state[ckdev->cols];
>> +
>> + ret = cros_ec_keyb_get_state(ckdev, kb_state);
>> + if (ret >= 0)
>> + cros_ec_keyb_process(ckdev, kb_state, ret);
>> +
>> + return NOTIFY_DONE;
>> +}
>> +
>> +/* On resume, clear any keys in the buffer */
>> +static int cros_ec_keyb_clear_keyboard(struct notifier_block *nb,
>> + unsigned long state, void *_notify)
>> +{
>> + struct cros_ec_keyb *ckdev = container_of(nb, struct cros_ec_keyb,
>> + wake_notifier);
>> + uint8_t old_state[ckdev->cols];
>> + uint8_t new_state[ckdev->cols];
>> + unsigned long duration;
>> + int i, ret;
>> +
>> + /*
>> + * Keep reading until we see that the scan state does not change.
>> + * That indicates that we are done.
>> + *
>> + * Assume that the EC keyscan buffer is at most 32 deep.
>> + */
>> + duration = jiffies;
>> + ret = cros_ec_keyb_get_state(ckdev, new_state);
>> + for (i = 1; !ret && i < 32; i++) {
>> + memcpy(old_state, new_state, sizeof(old_state));
>> + ret = cros_ec_keyb_get_state(ckdev, new_state);
>> + if (0 == memcmp(old_state, new_state, sizeof(old_state)))
>> + break;
>> + }
>> + duration = jiffies - duration;
>> + dev_info(ckdev->dev, "Discarded %d keyscan(s) in %dus\n", i,
>> + jiffies_to_usecs(duration));
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id cros_ec_kbc_of_match[] = {
>> + { .compatible = "google,cros-ec-keyb", },
>> + { },
>> +};
>> +
>> +static int cros_ec_keyb_probe(struct platform_device *pdev)
>> +{
>> + struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
>> + struct device *dev = ec->dev;
>> + struct cros_ec_keyb *ckdev = NULL;
>> + struct input_dev *idev = NULL;
>> + struct device_node *np;
>> + int err;
>> +
>> + np = of_find_matching_node(NULL, cros_ec_kbc_of_match);
>
> And if we don't find it?
Added error checking.
>
>> +
>> + ckdev = kzalloc(sizeof(*ckdev), GFP_KERNEL);
>> + if (!ckdev) {
>> + dev_err(dev, "cannot allocate memory for ckdev\n");
>> + return -ENOMEM;
>> + }
>> + pdev->dev.of_node = np;
>
> Huh? I'd expect the platform device be fully set up (including DT data)
> before the driver is called.
This is a child of the mfd driver cros_ec, so I don't think that
works. Or maybe I'm just not sure how to plumb it in so it is
automatic. Or maybe I just need to add the id to the device info
below?
>
>> + err = matrix_keypad_parse_of_params(&pdev->dev, &ckdev->rows,
>> + &ckdev->cols);
>> + if (err)
>> + goto fail_alloc_dev;
>> +
>> + idev = input_allocate_device();
>> + if (!idev) {
>> + err = -ENOMEM;
>> + dev_err(dev, "cannot allocate memory for input device\n");
>> + goto fail_alloc_dev;
>> + }
>> +
>> + ckdev->ec = ec;
>> + ckdev->notifier.notifier_call = cros_ec_keyb_work;
>> + ckdev->wake_notifier.notifier_call = cros_ec_keyb_clear_keyboard;
>> + ckdev->dev = dev;
>> + dev_set_drvdata(&pdev->dev, ckdev);
>> +
>> + idev->name = ec->get_name(ec);
>> + idev->phys = ec->get_phys_name(ec);
>> + __set_bit(EV_REP, idev->evbit);
>> +
>> + idev->id.bustype = BUS_VIRTUAL;
>> + idev->id.version = 1;
>> + idev->id.product = 0;
>> + idev->dev.parent = &pdev->dev;
>> + idev->open = cros_ec_keyb_open;
>> + idev->close = cros_ec_keyb_close;
>> +
>> + ckdev->ghost_filter = of_property_read_bool(np,
>> + "google,needs-ghost-filter");
>> +
>> + err = matrix_keypad_build_keymap(NULL, NULL, ckdev->rows, ckdev->cols,
>> + NULL, idev);
>> + if (err) {
>> + dev_err(dev, "cannot build key matrix\n");
>> + goto fail_matrix;
>> + }
>> +
>> + ckdev->row_shift = get_count_order(ckdev->cols);
>> + ckdev->old_state = kzalloc(idev->keycodemax, GFP_KERNEL);
>> + if (!ckdev->old_state) {
>> + dev_err(dev, "Cannot allocate memory for old_state\n");
>> + err = -ENOMEM;
>> + goto fail_old_state;
>> + }
>
> Not needed I believe.
Dropped.
>
>> +
>> + input_set_capability(idev, EV_MSC, MSC_SCAN);
>> + input_set_drvdata(idev, ckdev);
>> + ckdev->idev = idev;
>> + err = input_register_device(ckdev->idev);
>> + if (err) {
>> + dev_err(dev, "cannot register input device\n");
>> + goto fail_register;
>> + }
>> +
>> + return 0;
>> +
>> +fail_register:
>> + kfree(ckdev->old_state);
>> +fail_old_state:
>> + kfree(idev->keycode);
>> +fail_matrix:
>> + input_free_device(idev);
>> +fail_alloc_dev:
>> + kfree(ckdev);
>> + return err;
>> +}
>> +
>> +static int cros_ec_keyb_remove(struct platform_device *pdev)
>> +{
>> + struct cros_ec_keyb *ckdev = dev_get_drvdata(&pdev->dev);
>
> platform_get_drvdata() please.
Done
>
>> + struct input_dev *idev = ckdev->idev;
>> +
>> + /* I believe we leak a matrix_keymap here */
>
> How? It is devm-managed.
Original code might pre-date that. Removed comment.
>
>> + input_unregister_device(idev);
>> + kfree(ckdev->old_state);
>> + kfree(idev->keycode);
>
> And since it is devm-managed you should not free it yourself. Actually
> idev is most likely gone at this point already.
Yes, done.
>
>> + input_free_device(idev);
>
> Do not call input_free_device() after input_unregister_device().
Done
>
>> + kfree(ckdev);
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver cros_ec_keyb_driver = {
>> + .probe = cros_ec_keyb_probe,
>> + .remove = cros_ec_keyb_remove,
>> + .driver = {
>> + .name = "cros-ec-keyb",
>> + },
>> +};
>> +
>> +module_platform_driver(cros_ec_keyb_driver);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("ChromeOS EC keyboard driver");
>> +MODULE_ALIAS("platform:cros-ec-keyb");
>> --
>> 1.8.1
>>
>
> Thanks.
>
> --
> Dmitry
I'll send a new version so that the above is done, at least.
Regards,
Simon
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 6/6] Input: Add ChromeOS EC keyboard driver
2013-02-14 6:45 ` Simon Glass
@ 2013-02-14 17:31 ` Dmitry Torokhov
2013-02-16 3:56 ` Simon Glass
0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Torokhov @ 2013-02-14 17:31 UTC (permalink / raw)
To: Simon Glass
Cc: LKML, Samuel Ortiz, Luigi Semenzato, Vincent Palatin,
Grant Likely, Rob Herring, Rob Landley, Felipe Balbi,
Sourav Poddar, Tony Lindgren, Greg Kroah-Hartman, Mike A. Chan,
Jun Nakajima, Tom Keel, devicetree-discuss, linux-doc,
linux-input
On Wed, Feb 13, 2013 at 10:45:07PM -0800, Simon Glass wrote:
> >>
> >> +config KEYBOARD_CROS_EC
> >> + tristate "ChromeOS EC keyboard"
> >> + select INPUT_MATRIXKMAP
> >> + select MFD_CROS_EC
> >
> > Is this select safe? I.e. does MFD_CROS_EC depend on anything else?
>
> I'll remove it, since it isn't required, and it's true that it does
> need other things.
Instead of droppign the dependency completely I think it should "depens
on MFD_CROS_EC"
> >> +
> >> +static void cros_ec_keyb_close(struct input_dev *dev)
> >> +{
> >> + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
> >> +
> >> + blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
> >> + &ckdev->notifier);
> >> + blocking_notifier_chain_unregister(&ckdev->ec->wake_notifier,
> >> + &ckdev->wake_notifier);
> >
> > Why is this done via a notifier instead of regular resume method?
>
> Because we only call the notifer in resume when we were not waking on
> a keyboard event. We use it to flush the keyboard. It was a late
> change so there might be a better way, but this driver does not have a
> resume handler.
Right and the question is why does not it have resume handler and why
you inventing your own resume infrastructure instead of using the
standard one.
> >> +
> >> +static int cros_ec_keyb_probe(struct platform_device *pdev)
> >> +{
> >> + struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
> >> + struct device *dev = ec->dev;
> >> + struct cros_ec_keyb *ckdev = NULL;
> >> + struct input_dev *idev = NULL;
> >> + struct device_node *np;
> >> + int err;
> >> +
> >> + np = of_find_matching_node(NULL, cros_ec_kbc_of_match);
> >
> > And if we don't find it?
>
> Added error checking.
>
> >
> >> +
> >> + ckdev = kzalloc(sizeof(*ckdev), GFP_KERNEL);
> >> + if (!ckdev) {
> >> + dev_err(dev, "cannot allocate memory for ckdev\n");
> >> + return -ENOMEM;
> >> + }
> >> + pdev->dev.of_node = np;
> >
> > Huh? I'd expect the platform device be fully set up (including DT data)
> > before the driver is called.
>
> This is a child of the mfd driver cros_ec, so I don't think that
> works. Or maybe I'm just not sure how to plumb it in so it is
> automatic. Or maybe I just need to add the id to the device info
> below?
Who creates this device? Whoever does this should set up the
pdev->dev.of_node. This is not this driver's responsibility.
And then you add the id to the table below and matching is done
automatically.
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH v2 6/6] Input: Add ChromeOS EC keyboard driver
2013-02-14 17:31 ` Dmitry Torokhov
@ 2013-02-16 3:56 ` Simon Glass
0 siblings, 0 replies; 16+ messages in thread
From: Simon Glass @ 2013-02-16 3:56 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: LKML, Samuel Ortiz, Luigi Semenzato, Vincent Palatin,
Grant Likely, Rob Herring, Rob Landley, Felipe Balbi,
Sourav Poddar, Tony Lindgren, Greg Kroah-Hartman, Mike A. Chan,
Jun Nakajima, Tom Keel, devicetree-discuss, linux-doc,
linux-input
Hi Dmitry,
On Thu, Feb 14, 2013 at 9:31 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Wed, Feb 13, 2013 at 10:45:07PM -0800, Simon Glass wrote:
>> >>
>> >> +config KEYBOARD_CROS_EC
>> >> + tristate "ChromeOS EC keyboard"
>> >> + select INPUT_MATRIXKMAP
>> >> + select MFD_CROS_EC
>> >
>> > Is this select safe? I.e. does MFD_CROS_EC depend on anything else?
>>
>> I'll remove it, since it isn't required, and it's true that it does
>> need other things.
>
> Instead of droppign the dependency completely I think it should "depens
> on MFD_CROS_EC"
OK, done.
>
>> >> +
>> >> +static void cros_ec_keyb_close(struct input_dev *dev)
>> >> +{
>> >> + struct cros_ec_keyb *ckdev = input_get_drvdata(dev);
>> >> +
>> >> + blocking_notifier_chain_unregister(&ckdev->ec->event_notifier,
>> >> + &ckdev->notifier);
>> >> + blocking_notifier_chain_unregister(&ckdev->ec->wake_notifier,
>> >> + &ckdev->wake_notifier);
>> >
>> > Why is this done via a notifier instead of regular resume method?
>>
>> Because we only call the notifer in resume when we were not waking on
>> a keyboard event. We use it to flush the keyboard. It was a late
>> change so there might be a better way, but this driver does not have a
>> resume handler.
>
> Right and the question is why does not it have resume handler and why
> you inventing your own resume infrastructure instead of using the
> standard one.
I will fix that.
>
>> >> +
>> >> +static int cros_ec_keyb_probe(struct platform_device *pdev)
>> >> +{
>> >> + struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
>> >> + struct device *dev = ec->dev;
>> >> + struct cros_ec_keyb *ckdev = NULL;
>> >> + struct input_dev *idev = NULL;
>> >> + struct device_node *np;
>> >> + int err;
>> >> +
>> >> + np = of_find_matching_node(NULL, cros_ec_kbc_of_match);
>> >
>> > And if we don't find it?
>>
>> Added error checking.
>>
>> >
>> >> +
>> >> + ckdev = kzalloc(sizeof(*ckdev), GFP_KERNEL);
>> >> + if (!ckdev) {
>> >> + dev_err(dev, "cannot allocate memory for ckdev\n");
>> >> + return -ENOMEM;
>> >> + }
>> >> + pdev->dev.of_node = np;
>> >
>> > Huh? I'd expect the platform device be fully set up (including DT data)
>> > before the driver is called.
>>
>> This is a child of the mfd driver cros_ec, so I don't think that
>> works. Or maybe I'm just not sure how to plumb it in so it is
>> automatic. Or maybe I just need to add the id to the device info
>> below?
>
> Who creates this device? Whoever does this should set up the
> pdev->dev.of_node. This is not this driver's responsibility.
>
> And then you add the id to the table below and matching is done
> automatically.
OK I see - it comes from mfd and it is easy enough to make it do the
right thing.
Thanks for your comments and patience. I will send a new patch.
Regards,
Simon
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply [flat|nested] 16+ messages in thread