* Re: [PATCH v5] platform: chrome: Add ChromeOS EC ISHTP driver
From: Rushikesh S Kadam @ 2019-05-16 2:46 UTC (permalink / raw)
To: Enric Balletbo Serra
Cc: Enric Balletbo i Serra, benjamin.tissoires, jikos, Benson Leung,
Guenter Roeck, Srinivas Pandruvada, linux-kernel, linux-input,
Nick Crews, Jett Rink, Gwendal Grignou
In-Reply-To: <CAFqH_502a9rYYhUXFjUq5gmFH98JPxm-1CA3pW3XDtudnz-0tA@mail.gmail.com>
Hi Enric
On Wed, May 15, 2019 at 11:23:13PM +0200, Enric Balletbo Serra wrote:
> Missatge de Enric Balletbo i Serra <enric.balletbo@collabora.com> del
> dia dc., 15 de maig 2019 a les 15:00:
> >
> > Hi,
> >
> > On 4/5/19 15:34, Rushikesh S Kadam wrote:
> > > This driver implements a slim layer to enable the ChromeOS
> > > EC kernel stack (cros_ec) to communicate with ChromeOS EC
> > > firmware running on the Intel Integrated Sensor Hub (ISH).
> > >
> > > The driver registers a ChromeOS EC MFD device to connect
> > > with cros_ec kernel stack (upper layer), and it registers a
> > > client with the ISH Transport Protocol bus (lower layer) to
> > > talk with the ISH firwmare. See description of the ISHTP
> > > protocol at Documentation/hid/intel-ish-hid.txt
> > >
> > > Signed-off-by: Rushikesh S Kadam <rushikesh.s.kadam@intel.com>
> > > Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > > Reviewed-by: Jett Rink <jettrink@chromium.org>
> > > Tested-by: Jett Rink <jettrink@chromium.org>
> > > ---
> >
> > The following patch is queued to the for-next branch for the autobuilders to
> > play with, if all goes well I'll add the patch for 5.3 when current merge window
> > closes.
> >
>
> Actually, I reverted this patch and applied v6.
>
Thanks for your help! sorry for the extra effort.
Regards
Rushikesh
>
> > Thanks,
> > Enric
> >
> > >
> > > Submitting the patch to linux-input@ per the discussion here
> > > https://lkml.org/lkml/2019/5/2/339
> > >
> > > The patch is baselined to hid git tree, branch for-5.2/ish
> > > https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-5.2/ish
> > >
> > > v5
> > > - Submitting with all Acked-by & Tested-bys. No other changes.
> > >
> > > v4
> > > - Coding style related changes. No functional changes. Addresses
> > > review comments on v3.
> > >
> > > v3
> > > - Made several changes to improve code readability. Replaced
> > > multiple cl_data_to_dev(client_data) with dev variable. Use
> > > reverse Xmas tree for variable defintion where it made sense.
> > > Dropped few debug prints. Add docstring for function
> > > prepare_cros_ec_rx().
> > > - Fix code in function prepare_cros_ec_rx() under label
> > > end_cros_ec_dev_init_error.
> > > - Recycle buffer in process_recv() on failing to obtain the
> > > semaphore.
> > > - Increase ISHTP TX/RX ring buffer size to 8.
> > > - Alphabetically ordered CROS_EC_ISHTP entries in Makefile and
> > > Kconfig.
> > > - Updated commit message.
> > >
> > > v2
> > > - Dropped unused "reset" parameter in function cros_ec_init()
> > > - Change driver name to cros_ec_ishtp to be consistent with other
> > > references in the code.
> > > - Fixed a few typos.
> > >
> > > v1
> > > - Initial version
> > >
> > > drivers/platform/chrome/Kconfig | 13 +
> > > drivers/platform/chrome/Makefile | 1 +
> > > drivers/platform/chrome/cros_ec_ishtp.c | 763 ++++++++++++++++++++++++++++++++
> > > 3 files changed, 777 insertions(+)
> > > create mode 100644 drivers/platform/chrome/cros_ec_ishtp.c
> > >
> > > diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> > > index 16b1615..5848179 100644
> > > --- a/drivers/platform/chrome/Kconfig
> > > +++ b/drivers/platform/chrome/Kconfig
> > > @@ -62,6 +62,19 @@ config CROS_EC_I2C
> > > a checksum. Failing accesses will be retried three times to
> > > improve reliability.
> > >
> > > +config CROS_EC_ISHTP
> > > + tristate "ChromeOS Embedded Controller (ISHTP)"
> > > + depends on MFD_CROS_EC
> > > + depends on INTEL_ISH_HID
> > > + help
> > > + If you say Y here, you get support for talking to the ChromeOS EC
> > > + firmware running on Intel Integrated Sensor Hub (ISH), using the
> > > + ISH Transport protocol (ISH-TP). This uses a simple byte-level
> > > + protocol with a checksum.
> > > +
> > > + To compile this driver as a module, choose M here: the
> > > + module will be called cros_ec_ishtp.
> > > +
> > > config CROS_EC_SPI
> > > tristate "ChromeOS Embedded Controller (SPI)"
> > > depends on MFD_CROS_EC && SPI
> > > diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> > > index cd591bf..4efe102 100644
> > > --- a/drivers/platform/chrome/Makefile
> > > +++ b/drivers/platform/chrome/Makefile
> > > @@ -7,6 +7,7 @@ cros_ec_ctl-objs := cros_ec_sysfs.o cros_ec_lightbar.o \
> > > cros_ec_vbc.o cros_ec_debugfs.o
> > > obj-$(CONFIG_CROS_EC_CTL) += cros_ec_ctl.o
> > > obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o
> > > +obj-$(CONFIG_CROS_EC_ISHTP) += cros_ec_ishtp.o
> > > obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o
> > > cros_ec_lpcs-objs := cros_ec_lpc.o cros_ec_lpc_reg.o
> > > cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o
> > > diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
> > > new file mode 100644
> > > index 0000000..997503d
> > > --- /dev/null
> > > +++ b/drivers/platform/chrome/cros_ec_ishtp.c
> > > @@ -0,0 +1,763 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +// ISHTP interface for ChromeOS Embedded Controller
> > > +//
> > > +// Copyright (c) 2019, Intel Corporation.
> > > +//
> > > +// ISHTP client driver for talking to the Chrome OS EC firmware running
> > > +// on Intel Integrated Sensor Hub (ISH) using the ISH Transport protocol
> > > +// (ISH-TP).
> > > +
> > > +#include <linux/delay.h>
> > > +#include <linux/mfd/core.h>
> > > +#include <linux/mfd/cros_ec.h>
> > > +#include <linux/mfd/cros_ec_commands.h>
> > > +#include <linux/module.h>
> > > +#include <linux/pci.h>
> > > +#include <linux/intel-ish-client-if.h>
> > > +
> > > +/*
> > > + * ISH TX/RX ring buffer pool size
> > > + *
> > > + * The AP->ISH messages and corresponding ISH->AP responses are
> > > + * serialized. We need 1 TX and 1 RX buffer for these.
> > > + *
> > > + * The MKBP ISH->AP events are serialized. We need one additional RX
> > > + * buffer for them.
> > > + */
> > > +#define CROS_ISH_CL_TX_RING_SIZE 8
> > > +#define CROS_ISH_CL_RX_RING_SIZE 8
> > > +
> > > +/* ISH CrOS EC Host Commands */
> > > +enum cros_ec_ish_channel {
> > > + CROS_EC_COMMAND = 1, /* AP->ISH message */
> > > + CROS_MKBP_EVENT = 2, /* ISH->AP events */
> > > +};
> > > +
> > > +/*
> > > + * ISH firmware timeout for 1 message send failure is 1Hz, and the
> > > + * firmware will retry 2 times, so 3Hz is used for timeout.
> > > + */
> > > +#define ISHTP_SEND_TIMEOUT (3 * HZ)
> > > +
> > > +/* ISH Transport CrOS EC ISH client unique GUID */
> > > +static const guid_t cros_ish_guid =
> > > + GUID_INIT(0x7b7154d0, 0x56f4, 0x4bdc,
> > > + 0xb0, 0xd8, 0x9e, 0x7c, 0xda, 0xe0, 0xd6, 0xa0);
> > > +
> > > +struct header {
> > > + u8 channel;
> > > + u8 status;
> > > + u8 reserved[2];
> > > +} __packed;
> > > +
> > > +struct cros_ish_out_msg {
> > > + struct header hdr;
> > > + struct ec_host_request ec_request;
> > > +} __packed;
> > > +
> > > +struct cros_ish_in_msg {
> > > + struct header hdr;
> > > + struct ec_host_response ec_response;
> > > +} __packed;
> > > +
> > > +#define IN_MSG_EC_RESPONSE_PREAMBLE \
> > > + offsetof(struct cros_ish_in_msg, ec_response)
> > > +
> > > +#define OUT_MSG_EC_REQUEST_PREAMBLE \
> > > + offsetof(struct cros_ish_out_msg, ec_request)
> > > +
> > > +#define cl_data_to_dev(client_data) ishtp_device((client_data)->cl_device)
> > > +
> > > +/*
> > > + * The Read-Write Semaphore is used to prevent message TX or RX while
> > > + * the ishtp client is being initialized or undergoing reset.
> > > + *
> > > + * The readers are the kernel function calls responsible for IA->ISH
> > > + * and ISH->AP messaging.
> > > + *
> > > + * The writers are .reset() and .probe() function.
> > > + */
> > > +DECLARE_RWSEM(init_lock);
> > > +
> > > +/**
> > > + * struct response_info - Encapsulate firmware response related
> > > + * information for passing between function ish_send() and
> > > + * process_recv() callback.
> > > + *
> > > + * @data: Copy the data received from firmware here.
> > > + * @max_size: Max size allocated for the @data buffer. If the received
> > > + * data exceeds this value, we log an error.
> > > + * @size: Actual size of data received from firmware.
> > > + * @error: 0 for success, negative error code for a failure in process_recv().
> > > + * @received: Set to true on receiving a valid firmware response to host command
> > > + * @wait_queue: Wait queue for host to wait for firmware response.
> > > + */
> > > +struct response_info {
> > > + void *data;
> > > + size_t max_size;
> > > + size_t size;
> > > + int error;
> > > + bool received;
> > > + wait_queue_head_t wait_queue;
> > > +};
> > > +
> > > +/**
> > > + * struct ishtp_cl_data - Encapsulate per ISH TP Client.
> > > + *
> > > + * @cros_ish_cl: ISHTP firmware client instance.
> > > + * @cl_device: ISHTP client device instance.
> > > + * @response: Response info passing between ish_send() and process_recv().
> > > + * @work_ishtp_reset: Work queue reset handling.
> > > + * @work_ec_evt: Work queue for EC events.
> > > + * @ec_dev: CrOS EC MFD device.
> > > + *
> > > + * This structure is used to store per client data.
> > > + */
> > > +struct ishtp_cl_data {
> > > + struct ishtp_cl *cros_ish_cl;
> > > + struct ishtp_cl_device *cl_device;
> > > +
> > > + /*
> > > + * Used for passing firmware response information between
> > > + * ish_send() and process_recv() callback.
> > > + */
> > > + struct response_info response;
> > > +
> > > + struct work_struct work_ishtp_reset;
> > > + struct work_struct work_ec_evt;
> > > + struct cros_ec_device *ec_dev;
> > > +};
> > > +
> > > +/**
> > > + * ish_evt_handler - ISH to AP event handler
> > > + * @work: Work struct
> > > + */
> > > +static void ish_evt_handler(struct work_struct *work)
> > > +{
> > > + struct ishtp_cl_data *client_data =
> > > + container_of(work, struct ishtp_cl_data, work_ec_evt);
> > > + struct cros_ec_device *ec_dev = client_data->ec_dev;
> > > +
> > > + if (cros_ec_get_next_event(ec_dev, NULL) > 0) {
> > > + blocking_notifier_call_chain(&ec_dev->event_notifier,
> > > + 0, ec_dev);
> > > + }
> > > +}
> > > +
> > > +/**
> > > + * ish_send() - Send message from host to firmware
> > > + *
> > > + * @client_data: Client data instance
> > > + * @out_msg: Message buffer to be sent to firmware
> > > + * @out_size: Size of out going message
> > > + * @in_msg: Message buffer where the incoming data is copied. This buffer
> > > + * is allocated by calling
> > > + * @in_size: Max size of incoming message
> > > + *
> > > + * Return: Number of bytes copied in the in_msg on success, negative
> > > + * error code on failure.
> > > + */
> > > +static int ish_send(struct ishtp_cl_data *client_data,
> > > + u8 *out_msg, size_t out_size,
> > > + u8 *in_msg, size_t in_size)
> > > +{
> > > + int rv;
> > > + struct header *out_hdr = (struct header *)out_msg;
> > > + struct ishtp_cl *cros_ish_cl = client_data->cros_ish_cl;
> > > +
> > > + dev_dbg(cl_data_to_dev(client_data),
> > > + "%s: channel=%02u status=%02u\n",
> > > + __func__, out_hdr->channel, out_hdr->status);
> > > +
> > > + /* Setup for incoming response */
> > > + client_data->response.data = in_msg;
> > > + client_data->response.max_size = in_size;
> > > + client_data->response.error = 0;
> > > + client_data->response.received = false;
> > > +
> > > + rv = ishtp_cl_send(cros_ish_cl, out_msg, out_size);
> > > + if (rv) {
> > > + dev_err(cl_data_to_dev(client_data),
> > > + "ishtp_cl_send error %d\n", rv);
> > > + return rv;
> > > + }
> > > +
> > > + wait_event_interruptible_timeout(client_data->response.wait_queue,
> > > + client_data->response.received,
> > > + ISHTP_SEND_TIMEOUT);
> > > + if (!client_data->response.received) {
> > > + dev_err(cl_data_to_dev(client_data),
> > > + "Timed out for response to host message\n");
> > > + return -ETIMEDOUT;
> > > + }
> > > +
> > > + if (client_data->response.error < 0)
> > > + return client_data->response.error;
> > > +
> > > + return client_data->response.size;
> > > +}
> > > +
> > > +/**
> > > + * process_recv() - Received and parse incoming packet
> > > + * @cros_ish_cl: Client instance to get stats
> > > + * @rb_in_proc: Host interface message buffer
> > > + *
> > > + * Parse the incoming packet. If it is a response packet then it will
> > > + * update per instance flags and wake up the caller waiting to for the
> > > + * response. If it is an event packet then it will schedule event work.
> > > + */
> > > +static void process_recv(struct ishtp_cl *cros_ish_cl,
> > > + struct ishtp_cl_rb *rb_in_proc)
> > > +{
> > > + size_t data_len = rb_in_proc->buf_idx;
> > > + struct ishtp_cl_data *client_data =
> > > + ishtp_get_client_data(cros_ish_cl);
> > > + struct device *dev = cl_data_to_dev(client_data);
> > > + struct cros_ish_in_msg *in_msg =
> > > + (struct cros_ish_in_msg *)rb_in_proc->buffer.data;
> > > +
> > > + /* Proceed only if reset or init is not in progress */
> > > + if (!down_read_trylock(&init_lock)) {
> > > + /* Free the buffer */
> > > + ishtp_cl_io_rb_recycle(rb_in_proc);
> > > + dev_warn(dev,
> > > + "Host is not ready to receive incoming messages\n");
> > > + return;
> > > + }
> > > +
> > > + /*
> > > + * All firmware messages contain a header. Check the buffer size
> > > + * before accessing elements inside.
> > > + */
> > > + if (!rb_in_proc->buffer.data) {
> > > + dev_warn(dev, "rb_in_proc->buffer.data returned null");
> > > + client_data->response.error = -EBADMSG;
> > > + goto end_error;
> > > + }
> > > +
> > > + if (data_len < sizeof(struct header)) {
> > > + dev_err(dev, "data size %zu is less than header %zu\n",
> > > + data_len, sizeof(struct header));
> > > + client_data->response.error = -EMSGSIZE;
> > > + goto end_error;
> > > + }
> > > +
> > > + dev_dbg(dev, "channel=%02u status=%02u\n",
> > > + in_msg->hdr.channel, in_msg->hdr.status);
> > > +
> > > + switch (in_msg->hdr.channel) {
> > > + case CROS_EC_COMMAND:
> > > + /* Sanity check */
> > > + if (!client_data->response.data) {
> > > + dev_err(dev,
> > > + "Receiving buffer is null. Should be allocated by calling function\n");
> > > + client_data->response.error = -EINVAL;
> > > + goto error_wake_up;
> > > + }
> > > +
> > > + if (client_data->response.received) {
> > > + dev_err(dev,
> > > + "Previous firmware message not yet processed\n");
> > > + client_data->response.error = -EINVAL;
> > > + goto error_wake_up;
> > > + }
> > > +
> > > + if (data_len > client_data->response.max_size) {
> > > + dev_err(dev,
> > > + "Received buffer size %zu is larger than allocated buffer %zu\n",
> > > + data_len, client_data->response.max_size);
> > > + client_data->response.error = -EMSGSIZE;
> > > + goto error_wake_up;
> > > + }
> > > +
> > > + if (in_msg->hdr.status) {
> > > + dev_err(dev, "firmware returned status %d\n",
> > > + in_msg->hdr.status);
> > > + client_data->response.error = -EIO;
> > > + goto error_wake_up;
> > > + }
> > > +
> > > + /* Update the actual received buffer size */
> > > + client_data->response.size = data_len;
> > > +
> > > + /*
> > > + * Copy the buffer received in firmware response for the
> > > + * calling thread.
> > > + */
> > > + memcpy(client_data->response.data,
> > > + rb_in_proc->buffer.data, data_len);
> > > +
> > > + /* Set flag before waking up the caller */
> > > + client_data->response.received = true;
> > > +error_wake_up:
> > > + /* Wake the calling thread */
> > > + wake_up_interruptible(&client_data->response.wait_queue);
> > > +
> > > + break;
> > > +
> > > + case CROS_MKBP_EVENT:
> > > + /* The event system doesn't send any data in buffer */
> > > + schedule_work(&client_data->work_ec_evt);
> > > +
> > > + break;
> > > +
> > > + default:
> > > + dev_err(dev, "Invalid channel=%02d\n", in_msg->hdr.channel);
> > > + }
> > > +
> > > +end_error:
> > > + /* Free the buffer */
> > > + ishtp_cl_io_rb_recycle(rb_in_proc);
> > > +
> > > + up_read(&init_lock);
> > > +}
> > > +
> > > +/**
> > > + * ish_event_cb() - bus driver callback for incoming message
> > > + * @cl_device: ISHTP client device for which this message is targeted.
> > > + *
> > > + * Remove the packet from the list and process the message by calling
> > > + * process_recv.
> > > + */
> > > +static void ish_event_cb(struct ishtp_cl_device *cl_device)
> > > +{
> > > + struct ishtp_cl_rb *rb_in_proc;
> > > + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> > > +
> > > + while ((rb_in_proc = ishtp_cl_rx_get_rb(cros_ish_cl)) != NULL) {
> > > + /* Decide what to do with received data */
> > > + process_recv(cros_ish_cl, rb_in_proc);
> > > + }
> > > +}
> > > +
> > > +/**
> > > + * cros_ish_init() - Init function for ISHTP client
> > > + * @cros_ish_cl: ISHTP client instance
> > > + *
> > > + * This function complete the initializtion of the client.
> > > + *
> > > + * Return: 0 for success, negative error code for failure.
> > > + */
> > > +static int cros_ish_init(struct ishtp_cl *cros_ish_cl)
> > > +{
> > > + int rv;
> > > + struct ishtp_device *dev;
> > > + struct ishtp_fw_client *fw_client;
> > > + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> > > +
> > > + rv = ishtp_cl_link(cros_ish_cl);
> > > + if (rv) {
> > > + dev_err(cl_data_to_dev(client_data),
> > > + "ishtp_cl_link failed\n");
> > > + return rv;
> > > + }
> > > +
> > > + dev = ishtp_get_ishtp_device(cros_ish_cl);
> > > +
> > > + /* Connect to firmware client */
> > > + ishtp_set_tx_ring_size(cros_ish_cl, CROS_ISH_CL_TX_RING_SIZE);
> > > + ishtp_set_rx_ring_size(cros_ish_cl, CROS_ISH_CL_RX_RING_SIZE);
> > > +
> > > + fw_client = ishtp_fw_cl_get_client(dev, &cros_ish_guid);
> > > + if (!fw_client) {
> > > + dev_err(cl_data_to_dev(client_data),
> > > + "ish client uuid not found\n");
> > > + rv = -ENOENT;
> > > + goto err_cl_unlink;
> > > + }
> > > +
> > > + ishtp_cl_set_fw_client_id(cros_ish_cl,
> > > + ishtp_get_fw_client_id(fw_client));
> > > + ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_CONNECTING);
> > > +
> > > + rv = ishtp_cl_connect(cros_ish_cl);
> > > + if (rv) {
> > > + dev_err(cl_data_to_dev(client_data),
> > > + "client connect fail\n");
> > > + goto err_cl_unlink;
> > > + }
> > > +
> > > + ishtp_register_event_cb(client_data->cl_device, ish_event_cb);
> > > + return 0;
> > > +
> > > +err_cl_unlink:
> > > + ishtp_cl_unlink(cros_ish_cl);
> > > + return rv;
> > > +}
> > > +
> > > +/**
> > > + * cros_ish_deinit() - Deinit function for ISHTP client
> > > + * @cros_ish_cl: ISHTP client instance
> > > + *
> > > + * Unlink and free cros_ec client
> > > + */
> > > +static void cros_ish_deinit(struct ishtp_cl *cros_ish_cl)
> > > +{
> > > + ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_DISCONNECTING);
> > > + ishtp_cl_disconnect(cros_ish_cl);
> > > + ishtp_cl_unlink(cros_ish_cl);
> > > + ishtp_cl_flush_queues(cros_ish_cl);
> > > +
> > > + /* Disband and free all Tx and Rx client-level rings */
> > > + ishtp_cl_free(cros_ish_cl);
> > > +}
> > > +
> > > +/**
> > > + * prepare_cros_ec_rx() - Check & prepare receive buffer
> > > + * @ec_dev: CrOS EC MFD device.
> > > + * @in_msg: Incoming message buffer
> > > + * @msg: cros_ec command used to send & receive data
> > > + *
> > > + * Return: 0 for success, negative error code for failure.
> > > + *
> > > + * Check the received buffer. Convert to cros_ec_command format.
> > > + */
> > > +static int prepare_cros_ec_rx(struct cros_ec_device *ec_dev,
> > > + const struct cros_ish_in_msg *in_msg,
> > > + struct cros_ec_command *msg)
> > > +{
> > > + u8 sum = 0;
> > > + int i, rv, offset;
> > > +
> > > + /* Check response error code */
> > > + msg->result = in_msg->ec_response.result;
> > > + rv = cros_ec_check_result(ec_dev, msg);
> > > + if (rv < 0)
> > > + return rv;
> > > +
> > > + if (in_msg->ec_response.data_len > msg->insize) {
> > > + dev_err(ec_dev->dev, "Packet too long (%d bytes, expected %d)",
> > > + in_msg->ec_response.data_len, msg->insize);
> > > + return -ENOSPC;
> > > + }
> > > +
> > > + /* Copy response packet payload and compute checksum */
> > > + for (i = 0; i < sizeof(struct ec_host_response); i++)
> > > + sum += ((u8 *)in_msg)[IN_MSG_EC_RESPONSE_PREAMBLE + i];
> > > +
> > > + offset = sizeof(struct cros_ish_in_msg);
> > > + for (i = 0; i < in_msg->ec_response.data_len; i++)
> > > + sum += msg->data[i] = ((u8 *)in_msg)[offset + i];
> > > +
> > > + if (sum) {
> > > + dev_dbg(ec_dev->dev, "Bad received packet checksum %d\n", sum);
> > > + return -EBADMSG;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int cros_ec_pkt_xfer_ish(struct cros_ec_device *ec_dev,
> > > + struct cros_ec_command *msg)
> > > +{
> > > + int rv;
> > > + struct ishtp_cl *cros_ish_cl = ec_dev->priv;
> > > + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> > > + struct device *dev = cl_data_to_dev(client_data);
> > > + struct cros_ish_in_msg *in_msg = (struct cros_ish_in_msg *)ec_dev->din;
> > > + struct cros_ish_out_msg *out_msg =
> > > + (struct cros_ish_out_msg *)ec_dev->dout;
> > > + size_t in_size = sizeof(struct cros_ish_in_msg) + msg->insize;
> > > + size_t out_size = sizeof(struct cros_ish_out_msg) + msg->outsize;
> > > +
> > > + /* Proceed only if reset-init is not in progress */
> > > + if (!down_read_trylock(&init_lock)) {
> > > + dev_warn(dev,
> > > + "Host is not ready to send messages to ISH. Try again\n");
> > > + return -EAGAIN;
> > > + }
> > > +
> > > + /* Sanity checks */
> > > + if (in_size > ec_dev->din_size) {
> > > + dev_err(dev,
> > > + "Incoming payload size %zu is too large for ec_dev->din_size %d\n",
> > > + in_size, ec_dev->din_size);
> > > + return -EMSGSIZE;
> > > + }
> > > +
> > > + if (out_size > ec_dev->dout_size) {
> > > + dev_err(dev,
> > > + "Outgoing payload size %zu is too large for ec_dev->dout_size %d\n",
> > > + out_size, ec_dev->dout_size);
> > > + return -EMSGSIZE;
> > > + }
> > > +
> > > + /* Prepare the package to be sent over ISH TP */
> > > + out_msg->hdr.channel = CROS_EC_COMMAND;
> > > + out_msg->hdr.status = 0;
> > > +
> > > + ec_dev->dout += OUT_MSG_EC_REQUEST_PREAMBLE;
> > > + cros_ec_prepare_tx(ec_dev, msg);
> > > + ec_dev->dout -= OUT_MSG_EC_REQUEST_PREAMBLE;
> > > +
> > > + dev_dbg(dev,
> > > + "out_msg: struct_ver=0x%x checksum=0x%x command=0x%x command_ver=0x%x data_len=0x%x\n",
> > > + out_msg->ec_request.struct_version,
> > > + out_msg->ec_request.checksum,
> > > + out_msg->ec_request.command,
> > > + out_msg->ec_request.command_version,
> > > + out_msg->ec_request.data_len);
> > > +
> > > + /* Send command to ISH EC firmware and read response */
> > > + rv = ish_send(client_data,
> > > + (u8 *)out_msg, out_size,
> > > + (u8 *)in_msg, in_size);
> > > + if (rv < 0)
> > > + goto end_error;
> > > +
> > > + rv = prepare_cros_ec_rx(ec_dev, in_msg, msg);
> > > + if (rv)
> > > + goto end_error;
> > > +
> > > + rv = in_msg->ec_response.data_len;
> > > +
> > > + dev_dbg(dev,
> > > + "in_msg: struct_ver=0x%x checksum=0x%x result=0x%x data_len=0x%x\n",
> > > + in_msg->ec_response.struct_version,
> > > + in_msg->ec_response.checksum,
> > > + in_msg->ec_response.result,
> > > + in_msg->ec_response.data_len);
> > > +
> > > +end_error:
> > > + if (msg->command == EC_CMD_REBOOT_EC)
> > > + msleep(EC_REBOOT_DELAY_MS);
> > > +
> > > + up_read(&init_lock);
> > > +
> > > + return rv;
> > > +}
> > > +
> > > +static int cros_ec_dev_init(struct ishtp_cl_data *client_data)
> > > +{
> > > + struct cros_ec_device *ec_dev;
> > > + struct device *dev = cl_data_to_dev(client_data);
> > > +
> > > + ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
> > > + if (!ec_dev)
> > > + return -ENOMEM;
> > > +
> > > + client_data->ec_dev = ec_dev;
> > > + dev->driver_data = ec_dev;
> > > +
> > > + ec_dev->dev = dev;
> > > + ec_dev->priv = client_data->cros_ish_cl;
> > > + ec_dev->cmd_xfer = NULL;
> > > + ec_dev->pkt_xfer = cros_ec_pkt_xfer_ish;
> > > + ec_dev->phys_name = dev_name(dev);
> > > + ec_dev->din_size = sizeof(struct cros_ish_in_msg) +
> > > + sizeof(struct ec_response_get_protocol_info);
> > > + ec_dev->dout_size = sizeof(struct cros_ish_out_msg);
> > > +
> > > + return cros_ec_register(ec_dev);
> > > +}
> > > +
> > > +static void reset_handler(struct work_struct *work)
> > > +{
> > > + int rv;
> > > + struct device *dev;
> > > + struct ishtp_cl *cros_ish_cl;
> > > + struct ishtp_cl_device *cl_device;
> > > + struct ishtp_cl_data *client_data =
> > > + container_of(work, struct ishtp_cl_data, work_ishtp_reset);
> > > +
> > > + /* Lock for reset to complete */
> > > + down_write(&init_lock);
> > > +
> > > + cros_ish_cl = client_data->cros_ish_cl;
> > > + cl_device = client_data->cl_device;
> > > +
> > > + /* Unlink, flush queues & start again */
> > > + ishtp_cl_unlink(cros_ish_cl);
> > > + ishtp_cl_flush_queues(cros_ish_cl);
> > > + ishtp_cl_free(cros_ish_cl);
> > > +
> > > + cros_ish_cl = ishtp_cl_allocate(cl_device);
> > > + if (!cros_ish_cl) {
> > > + up_write(&init_lock);
> > > + return;
> > > + }
> > > +
> > > + ishtp_set_drvdata(cl_device, cros_ish_cl);
> > > + ishtp_set_client_data(cros_ish_cl, client_data);
> > > + client_data->cros_ish_cl = cros_ish_cl;
> > > +
> > > + rv = cros_ish_init(cros_ish_cl);
> > > + if (rv) {
> > > + ishtp_cl_free(cros_ish_cl);
> > > + dev_err(cl_data_to_dev(client_data), "Reset Failed\n");
> > > + up_write(&init_lock);
> > > + return;
> > > + }
> > > +
> > > + /* Refresh ec_dev device pointers */
> > > + client_data->ec_dev->priv = client_data->cros_ish_cl;
> > > + dev = cl_data_to_dev(client_data);
> > > + dev->driver_data = client_data->ec_dev;
> > > +
> > > + dev_info(cl_data_to_dev(client_data), "Chrome EC ISH reset done\n");
> > > +
> > > + up_write(&init_lock);
> > > +}
> > > +
> > > +/**
> > > + * cros_ec_ishtp_probe() - ISHTP client driver probe callback
> > > + * @cl_device: ISHTP client device instance
> > > + *
> > > + * Return: 0 for success, negative error code for failure.
> > > + */
> > > +static int cros_ec_ishtp_probe(struct ishtp_cl_device *cl_device)
> > > +{
> > > + int rv;
> > > + struct ishtp_cl *cros_ish_cl;
> > > + struct ishtp_cl_data *client_data =
> > > + devm_kzalloc(ishtp_device(cl_device),
> > > + sizeof(*client_data), GFP_KERNEL);
> > > + if (!client_data)
> > > + return -ENOMEM;
> > > +
> > > + /* Lock for initialization to complete */
> > > + down_write(&init_lock);
> > > +
> > > + cros_ish_cl = ishtp_cl_allocate(cl_device);
> > > + if (!cros_ish_cl) {
> > > + rv = -ENOMEM;
> > > + goto end_ishtp_cl_alloc_error;
> > > + }
> > > +
> > > + ishtp_set_drvdata(cl_device, cros_ish_cl);
> > > + ishtp_set_client_data(cros_ish_cl, client_data);
> > > + client_data->cros_ish_cl = cros_ish_cl;
> > > + client_data->cl_device = cl_device;
> > > +
> > > + init_waitqueue_head(&client_data->response.wait_queue);
> > > +
> > > + INIT_WORK(&client_data->work_ishtp_reset,
> > > + reset_handler);
> > > + INIT_WORK(&client_data->work_ec_evt,
> > > + ish_evt_handler);
> > > +
> > > + rv = cros_ish_init(cros_ish_cl);
> > > + if (rv)
> > > + goto end_ishtp_cl_init_error;
> > > +
> > > + ishtp_get_device(cl_device);
> > > +
> > > + up_write(&init_lock);
> > > +
> > > + /* Register croc_ec_dev mfd */
> > > + rv = cros_ec_dev_init(client_data);
> > > + if (rv)
> > > + goto end_cros_ec_dev_init_error;
> > > +
> > > + return 0;
> > > +
> > > +end_cros_ec_dev_init_error:
> > > + ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_DISCONNECTING);
> > > + ishtp_cl_disconnect(cros_ish_cl);
> > > + ishtp_cl_unlink(cros_ish_cl);
> > > + ishtp_cl_flush_queues(cros_ish_cl);
> > > + ishtp_put_device(cl_device);
> > > +end_ishtp_cl_init_error:
> > > + ishtp_cl_free(cros_ish_cl);
> > > +end_ishtp_cl_alloc_error:
> > > + up_write(&init_lock);
> > > + return rv;
> > > +}
> > > +
> > > +/**
> > > + * cros_ec_ishtp_remove() - ISHTP client driver remove callback
> > > + * @cl_device: ISHTP client device instance
> > > + *
> > > + * Return: 0
> > > + */
> > > +static int cros_ec_ishtp_remove(struct ishtp_cl_device *cl_device)
> > > +{
> > > + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> > > + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> > > +
> > > + cancel_work_sync(&client_data->work_ishtp_reset);
> > > + cancel_work_sync(&client_data->work_ec_evt);
> > > + cros_ish_deinit(cros_ish_cl);
> > > + ishtp_put_device(cl_device);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * cros_ec_ishtp_reset() - ISHTP client driver reset callback
> > > + * @cl_device: ISHTP client device instance
> > > + *
> > > + * Return: 0
> > > + */
> > > +static int cros_ec_ishtp_reset(struct ishtp_cl_device *cl_device)
> > > +{
> > > + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> > > + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> > > +
> > > + schedule_work(&client_data->work_ishtp_reset);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +/**
> > > + * cros_ec_ishtp_suspend() - ISHTP client driver suspend callback
> > > + * @device: device instance
> > > + *
> > > + * Return: 0 for success, negative error code for failure.
> > > + */
> > > +static int __maybe_unused cros_ec_ishtp_suspend(struct device *device)
> > > +{
> > > + struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
> > > + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> > > + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> > > +
> > > + return cros_ec_suspend(client_data->ec_dev);
> > > +}
> > > +
> > > +/**
> > > + * cros_ec_ishtp_resume() - ISHTP client driver resume callback
> > > + * @device: device instance
> > > + *
> > > + * Return: 0 for success, negative error code for failure.
> > > + */
> > > +static int __maybe_unused cros_ec_ishtp_resume(struct device *device)
> > > +{
> > > + struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
> > > + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> > > + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> > > +
> > > + return cros_ec_resume(client_data->ec_dev);
> > > +}
> > > +
> > > +static SIMPLE_DEV_PM_OPS(cros_ec_ishtp_pm_ops, cros_ec_ishtp_suspend,
> > > + cros_ec_ishtp_resume);
> > > +
> > > +static struct ishtp_cl_driver cros_ec_ishtp_driver = {
> > > + .name = "cros_ec_ishtp",
> > > + .guid = &cros_ish_guid,
> > > + .probe = cros_ec_ishtp_probe,
> > > + .remove = cros_ec_ishtp_remove,
> > > + .reset = cros_ec_ishtp_reset,
> > > + .driver = {
> > > + .pm = &cros_ec_ishtp_pm_ops,
> > > + },
> > > +};
> > > +
> > > +static int __init cros_ec_ishtp_mod_init(void)
> > > +{
> > > + return ishtp_cl_driver_register(&cros_ec_ishtp_driver, THIS_MODULE);
> > > +}
> > > +
> > > +static void __exit cros_ec_ishtp_mod_exit(void)
> > > +{
> > > + ishtp_cl_driver_unregister(&cros_ec_ishtp_driver);
> > > +}
> > > +
> > > +module_init(cros_ec_ishtp_mod_init);
> > > +module_exit(cros_ec_ishtp_mod_exit);
> > > +
> > > +MODULE_DESCRIPTION("ChromeOS EC ISHTP Client Driver");
> > > +MODULE_AUTHOR("Rushikesh S Kadam <rushikesh.s.kadam@intel.com>");
> > > +
> > > +MODULE_LICENSE("GPL v2");
> > > +MODULE_ALIAS("ishtp:*");
> > >
--
^ permalink raw reply
* Re: [PATCH V1] elan_i2c: Increment wakeup count if wake source.
From: Dmitry Torokhov @ 2019-05-15 23:15 UTC (permalink / raw)
To: Ravi Chandra Sadineni
Cc: 廖崇榮, Benjamin Tissoires, Abhishek Bhardwaj,
Todd Broch, lkml, linux-input@vger.kernel.org
In-Reply-To: <CAEZbON4Z5GKYvMZJ8ojko_f1xzv2rf4uR6cDz2LMxu+XvzTzog@mail.gmail.com>
On Wed, May 15, 2019 at 09:17:59AM -0700, Ravi Chandra Sadineni wrote:
> Hi Dmitry,
>
> On Mon, May 13, 2019 at 4:29 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Hi Ravi,
> >
> > On Mon, May 13, 2019 at 3:06 PM Ravi Chandra Sadineni
> > <ravisadineni@chromium.org> wrote:
> > >
> > > Notify the PM core that this dev is the wake source. This helps
> > > userspace daemon tracking the wake source to identify the origin of the
> > > wake.
> >
> > I wonder if we could do that form the i2c core instead of individual drivers?
> I am sorry, I don't see a way how this could be done.
Sorry, brain fart on my part. Applied, thank you.
> >
> > >
> > > Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> > > ---
> > > drivers/input/mouse/elan_i2c_core.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> > > index f9525d6f0bfe..2c0561e20b7f 100644
> > > --- a/drivers/input/mouse/elan_i2c_core.c
> > > +++ b/drivers/input/mouse/elan_i2c_core.c
> > > @@ -981,6 +981,8 @@ static irqreturn_t elan_isr(int irq, void *dev_id)
> > > if (error)
> > > goto out;
> > >
> > > + pm_wakeup_event(dev, 0);
> > > +
> > > switch (report[ETP_REPORT_ID_OFFSET]) {
> > > case ETP_REPORT_ID:
> > > elan_report_absolute(data, report);
> > > --
> > > 2.20.1
> > >
> >
> > Thanks.
> >
> > --
> > Dmitry
>
> Thanks,
> Ravi
--
Dmitry
^ permalink raw reply
* Re: [PATCH v6] platform: chrome: Add ChromeOS EC ISHTP driver
From: Enric Balletbo Serra @ 2019-05-15 21:24 UTC (permalink / raw)
To: Rushikesh S Kadam
Cc: benjamin.tissoires, jikos, Benson Leung, Enric Balletbo i Serra,
Guenter Roeck, Srinivas Pandruvada, linux-kernel, linux-input,
Nick Crews, Jett Rink, Gwendal Grignou
In-Reply-To: <1557916721-31315-1-git-send-email-rushikesh.s.kadam@intel.com>
Hi,
Missatge de Rushikesh S Kadam <rushikesh.s.kadam@intel.com> del dia
dc., 15 de maig 2019 a les 12:41:
>
> This driver implements a slim layer to enable the ChromeOS
> EC kernel stack (cros_ec) to communicate with ChromeOS EC
> firmware running on the Intel Integrated Sensor Hub (ISH).
>
> The driver registers a ChromeOS EC MFD device to connect
> with cros_ec kernel stack (upper layer), and it registers a
> client with the ISH Transport Protocol bus (lower layer) to
> talk with the ISH firwmare. See description of the ISHTP
> protocol at Documentation/hid/intel-ish-hid.txt
>
> Signed-off-by: Rushikesh S Kadam <rushikesh.s.kadam@intel.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Reviewed-by: Jett Rink <jettrink@chromium.org>
> Tested-by: Jett Rink <jettrink@chromium.org>
> ---
The following patch is queued to the for-next branch for the autobuilders to
play with, if all goes well I'll add the patch for 5.3 when current merge window
closes.
Thanks,
Enric
>
> Submitting the patch to linux-input@ per the discussion here
> https://lkml.org/lkml/2019/5/2/339
>
> The patch is baselined to hid git tree, branch for-5.2/ish
> https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-5.2/ish
>
> v6
> - Moved the sanity checks in cros_ec_pkt_xfer_ish() to before
> the point we take the lock (Bug fix).
>
> v5
> - Submitting with all Acked-by & Tested-bys. No other changes.
>
> v4
> - Coding style related changes. No functional changes. Addresses
> review comments on v3.
>
> v3
> - Made several changes to improve code readability. Replaced
> multiple cl_data_to_dev(client_data) with dev variable. Use
> reverse Xmas tree for variable defintion where it made sense.
> Dropped few debug prints. Add docstring for function
> prepare_cros_ec_rx().
> - Fix code in function prepare_cros_ec_rx() under label
> end_cros_ec_dev_init_error.
> - Recycle buffer in process_recv() on failing to obtain the
> semaphore.
> - Increase ISHTP TX/RX ring buffer size to 8.
> - Alphabetically ordered CROS_EC_ISHTP entries in Makefile and
> Kconfig.
> - Updated commit message.
>
> v2
> - Dropped unused "reset" parameter in function cros_ec_init()
> - Change driver name to cros_ec_ishtp to be consistent with other
> references in the code.
> - Fixed a few typos.
>
> v1
> - Initial version
>
> drivers/platform/chrome/Kconfig | 13 +
> drivers/platform/chrome/Makefile | 1 +
> drivers/platform/chrome/cros_ec_ishtp.c | 763 ++++++++++++++++++++++++++++++++
> 3 files changed, 777 insertions(+)
> create mode 100644 drivers/platform/chrome/cros_ec_ishtp.c
>
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 16b1615..5848179 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -62,6 +62,19 @@ config CROS_EC_I2C
> a checksum. Failing accesses will be retried three times to
> improve reliability.
>
> +config CROS_EC_ISHTP
> + tristate "ChromeOS Embedded Controller (ISHTP)"
> + depends on MFD_CROS_EC
> + depends on INTEL_ISH_HID
> + help
> + If you say Y here, you get support for talking to the ChromeOS EC
> + firmware running on Intel Integrated Sensor Hub (ISH), using the
> + ISH Transport protocol (ISH-TP). This uses a simple byte-level
> + protocol with a checksum.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called cros_ec_ishtp.
> +
> config CROS_EC_SPI
> tristate "ChromeOS Embedded Controller (SPI)"
> depends on MFD_CROS_EC && SPI
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index cd591bf..4efe102 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -7,6 +7,7 @@ cros_ec_ctl-objs := cros_ec_sysfs.o cros_ec_lightbar.o \
> cros_ec_vbc.o cros_ec_debugfs.o
> obj-$(CONFIG_CROS_EC_CTL) += cros_ec_ctl.o
> obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o
> +obj-$(CONFIG_CROS_EC_ISHTP) += cros_ec_ishtp.o
> obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o
> cros_ec_lpcs-objs := cros_ec_lpc.o cros_ec_lpc_reg.o
> cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o
> diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
> new file mode 100644
> index 0000000..e504d25
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_ishtp.c
> @@ -0,0 +1,763 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// ISHTP interface for ChromeOS Embedded Controller
> +//
> +// Copyright (c) 2019, Intel Corporation.
> +//
> +// ISHTP client driver for talking to the Chrome OS EC firmware running
> +// on Intel Integrated Sensor Hub (ISH) using the ISH Transport protocol
> +// (ISH-TP).
> +
> +#include <linux/delay.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/intel-ish-client-if.h>
> +
> +/*
> + * ISH TX/RX ring buffer pool size
> + *
> + * The AP->ISH messages and corresponding ISH->AP responses are
> + * serialized. We need 1 TX and 1 RX buffer for these.
> + *
> + * The MKBP ISH->AP events are serialized. We need one additional RX
> + * buffer for them.
> + */
> +#define CROS_ISH_CL_TX_RING_SIZE 8
> +#define CROS_ISH_CL_RX_RING_SIZE 8
> +
> +/* ISH CrOS EC Host Commands */
> +enum cros_ec_ish_channel {
> + CROS_EC_COMMAND = 1, /* AP->ISH message */
> + CROS_MKBP_EVENT = 2, /* ISH->AP events */
> +};
> +
> +/*
> + * ISH firmware timeout for 1 message send failure is 1Hz, and the
> + * firmware will retry 2 times, so 3Hz is used for timeout.
> + */
> +#define ISHTP_SEND_TIMEOUT (3 * HZ)
> +
> +/* ISH Transport CrOS EC ISH client unique GUID */
> +static const guid_t cros_ish_guid =
> + GUID_INIT(0x7b7154d0, 0x56f4, 0x4bdc,
> + 0xb0, 0xd8, 0x9e, 0x7c, 0xda, 0xe0, 0xd6, 0xa0);
> +
> +struct header {
> + u8 channel;
> + u8 status;
> + u8 reserved[2];
> +} __packed;
> +
> +struct cros_ish_out_msg {
> + struct header hdr;
> + struct ec_host_request ec_request;
> +} __packed;
> +
> +struct cros_ish_in_msg {
> + struct header hdr;
> + struct ec_host_response ec_response;
> +} __packed;
> +
> +#define IN_MSG_EC_RESPONSE_PREAMBLE \
> + offsetof(struct cros_ish_in_msg, ec_response)
> +
> +#define OUT_MSG_EC_REQUEST_PREAMBLE \
> + offsetof(struct cros_ish_out_msg, ec_request)
> +
> +#define cl_data_to_dev(client_data) ishtp_device((client_data)->cl_device)
> +
> +/*
> + * The Read-Write Semaphore is used to prevent message TX or RX while
> + * the ishtp client is being initialized or undergoing reset.
> + *
> + * The readers are the kernel function calls responsible for IA->ISH
> + * and ISH->AP messaging.
> + *
> + * The writers are .reset() and .probe() function.
> + */
> +DECLARE_RWSEM(init_lock);
> +
> +/**
> + * struct response_info - Encapsulate firmware response related
> + * information for passing between function ish_send() and
> + * process_recv() callback.
> + *
> + * @data: Copy the data received from firmware here.
> + * @max_size: Max size allocated for the @data buffer. If the received
> + * data exceeds this value, we log an error.
> + * @size: Actual size of data received from firmware.
> + * @error: 0 for success, negative error code for a failure in process_recv().
> + * @received: Set to true on receiving a valid firmware response to host command
> + * @wait_queue: Wait queue for host to wait for firmware response.
> + */
> +struct response_info {
> + void *data;
> + size_t max_size;
> + size_t size;
> + int error;
> + bool received;
> + wait_queue_head_t wait_queue;
> +};
> +
> +/**
> + * struct ishtp_cl_data - Encapsulate per ISH TP Client.
> + *
> + * @cros_ish_cl: ISHTP firmware client instance.
> + * @cl_device: ISHTP client device instance.
> + * @response: Response info passing between ish_send() and process_recv().
> + * @work_ishtp_reset: Work queue reset handling.
> + * @work_ec_evt: Work queue for EC events.
> + * @ec_dev: CrOS EC MFD device.
> + *
> + * This structure is used to store per client data.
> + */
> +struct ishtp_cl_data {
> + struct ishtp_cl *cros_ish_cl;
> + struct ishtp_cl_device *cl_device;
> +
> + /*
> + * Used for passing firmware response information between
> + * ish_send() and process_recv() callback.
> + */
> + struct response_info response;
> +
> + struct work_struct work_ishtp_reset;
> + struct work_struct work_ec_evt;
> + struct cros_ec_device *ec_dev;
> +};
> +
> +/**
> + * ish_evt_handler - ISH to AP event handler
> + * @work: Work struct
> + */
> +static void ish_evt_handler(struct work_struct *work)
> +{
> + struct ishtp_cl_data *client_data =
> + container_of(work, struct ishtp_cl_data, work_ec_evt);
> + struct cros_ec_device *ec_dev = client_data->ec_dev;
> +
> + if (cros_ec_get_next_event(ec_dev, NULL) > 0) {
> + blocking_notifier_call_chain(&ec_dev->event_notifier,
> + 0, ec_dev);
> + }
> +}
> +
> +/**
> + * ish_send() - Send message from host to firmware
> + *
> + * @client_data: Client data instance
> + * @out_msg: Message buffer to be sent to firmware
> + * @out_size: Size of out going message
> + * @in_msg: Message buffer where the incoming data is copied. This buffer
> + * is allocated by calling
> + * @in_size: Max size of incoming message
> + *
> + * Return: Number of bytes copied in the in_msg on success, negative
> + * error code on failure.
> + */
> +static int ish_send(struct ishtp_cl_data *client_data,
> + u8 *out_msg, size_t out_size,
> + u8 *in_msg, size_t in_size)
> +{
> + int rv;
> + struct header *out_hdr = (struct header *)out_msg;
> + struct ishtp_cl *cros_ish_cl = client_data->cros_ish_cl;
> +
> + dev_dbg(cl_data_to_dev(client_data),
> + "%s: channel=%02u status=%02u\n",
> + __func__, out_hdr->channel, out_hdr->status);
> +
> + /* Setup for incoming response */
> + client_data->response.data = in_msg;
> + client_data->response.max_size = in_size;
> + client_data->response.error = 0;
> + client_data->response.received = false;
> +
> + rv = ishtp_cl_send(cros_ish_cl, out_msg, out_size);
> + if (rv) {
> + dev_err(cl_data_to_dev(client_data),
> + "ishtp_cl_send error %d\n", rv);
> + return rv;
> + }
> +
> + wait_event_interruptible_timeout(client_data->response.wait_queue,
> + client_data->response.received,
> + ISHTP_SEND_TIMEOUT);
> + if (!client_data->response.received) {
> + dev_err(cl_data_to_dev(client_data),
> + "Timed out for response to host message\n");
> + return -ETIMEDOUT;
> + }
> +
> + if (client_data->response.error < 0)
> + return client_data->response.error;
> +
> + return client_data->response.size;
> +}
> +
> +/**
> + * process_recv() - Received and parse incoming packet
> + * @cros_ish_cl: Client instance to get stats
> + * @rb_in_proc: Host interface message buffer
> + *
> + * Parse the incoming packet. If it is a response packet then it will
> + * update per instance flags and wake up the caller waiting to for the
> + * response. If it is an event packet then it will schedule event work.
> + */
> +static void process_recv(struct ishtp_cl *cros_ish_cl,
> + struct ishtp_cl_rb *rb_in_proc)
> +{
> + size_t data_len = rb_in_proc->buf_idx;
> + struct ishtp_cl_data *client_data =
> + ishtp_get_client_data(cros_ish_cl);
> + struct device *dev = cl_data_to_dev(client_data);
> + struct cros_ish_in_msg *in_msg =
> + (struct cros_ish_in_msg *)rb_in_proc->buffer.data;
> +
> + /* Proceed only if reset or init is not in progress */
> + if (!down_read_trylock(&init_lock)) {
> + /* Free the buffer */
> + ishtp_cl_io_rb_recycle(rb_in_proc);
> + dev_warn(dev,
> + "Host is not ready to receive incoming messages\n");
> + return;
> + }
> +
> + /*
> + * All firmware messages contain a header. Check the buffer size
> + * before accessing elements inside.
> + */
> + if (!rb_in_proc->buffer.data) {
> + dev_warn(dev, "rb_in_proc->buffer.data returned null");
> + client_data->response.error = -EBADMSG;
> + goto end_error;
> + }
> +
> + if (data_len < sizeof(struct header)) {
> + dev_err(dev, "data size %zu is less than header %zu\n",
> + data_len, sizeof(struct header));
> + client_data->response.error = -EMSGSIZE;
> + goto end_error;
> + }
> +
> + dev_dbg(dev, "channel=%02u status=%02u\n",
> + in_msg->hdr.channel, in_msg->hdr.status);
> +
> + switch (in_msg->hdr.channel) {
> + case CROS_EC_COMMAND:
> + /* Sanity check */
> + if (!client_data->response.data) {
> + dev_err(dev,
> + "Receiving buffer is null. Should be allocated by calling function\n");
> + client_data->response.error = -EINVAL;
> + goto error_wake_up;
> + }
> +
> + if (client_data->response.received) {
> + dev_err(dev,
> + "Previous firmware message not yet processed\n");
> + client_data->response.error = -EINVAL;
> + goto error_wake_up;
> + }
> +
> + if (data_len > client_data->response.max_size) {
> + dev_err(dev,
> + "Received buffer size %zu is larger than allocated buffer %zu\n",
> + data_len, client_data->response.max_size);
> + client_data->response.error = -EMSGSIZE;
> + goto error_wake_up;
> + }
> +
> + if (in_msg->hdr.status) {
> + dev_err(dev, "firmware returned status %d\n",
> + in_msg->hdr.status);
> + client_data->response.error = -EIO;
> + goto error_wake_up;
> + }
> +
> + /* Update the actual received buffer size */
> + client_data->response.size = data_len;
> +
> + /*
> + * Copy the buffer received in firmware response for the
> + * calling thread.
> + */
> + memcpy(client_data->response.data,
> + rb_in_proc->buffer.data, data_len);
> +
> + /* Set flag before waking up the caller */
> + client_data->response.received = true;
> +error_wake_up:
> + /* Wake the calling thread */
> + wake_up_interruptible(&client_data->response.wait_queue);
> +
> + break;
> +
> + case CROS_MKBP_EVENT:
> + /* The event system doesn't send any data in buffer */
> + schedule_work(&client_data->work_ec_evt);
> +
> + break;
> +
> + default:
> + dev_err(dev, "Invalid channel=%02d\n", in_msg->hdr.channel);
> + }
> +
> +end_error:
> + /* Free the buffer */
> + ishtp_cl_io_rb_recycle(rb_in_proc);
> +
> + up_read(&init_lock);
> +}
> +
> +/**
> + * ish_event_cb() - bus driver callback for incoming message
> + * @cl_device: ISHTP client device for which this message is targeted.
> + *
> + * Remove the packet from the list and process the message by calling
> + * process_recv.
> + */
> +static void ish_event_cb(struct ishtp_cl_device *cl_device)
> +{
> + struct ishtp_cl_rb *rb_in_proc;
> + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> +
> + while ((rb_in_proc = ishtp_cl_rx_get_rb(cros_ish_cl)) != NULL) {
> + /* Decide what to do with received data */
> + process_recv(cros_ish_cl, rb_in_proc);
> + }
> +}
> +
> +/**
> + * cros_ish_init() - Init function for ISHTP client
> + * @cros_ish_cl: ISHTP client instance
> + *
> + * This function complete the initializtion of the client.
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int cros_ish_init(struct ishtp_cl *cros_ish_cl)
> +{
> + int rv;
> + struct ishtp_device *dev;
> + struct ishtp_fw_client *fw_client;
> + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> +
> + rv = ishtp_cl_link(cros_ish_cl);
> + if (rv) {
> + dev_err(cl_data_to_dev(client_data),
> + "ishtp_cl_link failed\n");
> + return rv;
> + }
> +
> + dev = ishtp_get_ishtp_device(cros_ish_cl);
> +
> + /* Connect to firmware client */
> + ishtp_set_tx_ring_size(cros_ish_cl, CROS_ISH_CL_TX_RING_SIZE);
> + ishtp_set_rx_ring_size(cros_ish_cl, CROS_ISH_CL_RX_RING_SIZE);
> +
> + fw_client = ishtp_fw_cl_get_client(dev, &cros_ish_guid);
> + if (!fw_client) {
> + dev_err(cl_data_to_dev(client_data),
> + "ish client uuid not found\n");
> + rv = -ENOENT;
> + goto err_cl_unlink;
> + }
> +
> + ishtp_cl_set_fw_client_id(cros_ish_cl,
> + ishtp_get_fw_client_id(fw_client));
> + ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_CONNECTING);
> +
> + rv = ishtp_cl_connect(cros_ish_cl);
> + if (rv) {
> + dev_err(cl_data_to_dev(client_data),
> + "client connect fail\n");
> + goto err_cl_unlink;
> + }
> +
> + ishtp_register_event_cb(client_data->cl_device, ish_event_cb);
> + return 0;
> +
> +err_cl_unlink:
> + ishtp_cl_unlink(cros_ish_cl);
> + return rv;
> +}
> +
> +/**
> + * cros_ish_deinit() - Deinit function for ISHTP client
> + * @cros_ish_cl: ISHTP client instance
> + *
> + * Unlink and free cros_ec client
> + */
> +static void cros_ish_deinit(struct ishtp_cl *cros_ish_cl)
> +{
> + ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_DISCONNECTING);
> + ishtp_cl_disconnect(cros_ish_cl);
> + ishtp_cl_unlink(cros_ish_cl);
> + ishtp_cl_flush_queues(cros_ish_cl);
> +
> + /* Disband and free all Tx and Rx client-level rings */
> + ishtp_cl_free(cros_ish_cl);
> +}
> +
> +/**
> + * prepare_cros_ec_rx() - Check & prepare receive buffer
> + * @ec_dev: CrOS EC MFD device.
> + * @in_msg: Incoming message buffer
> + * @msg: cros_ec command used to send & receive data
> + *
> + * Return: 0 for success, negative error code for failure.
> + *
> + * Check the received buffer. Convert to cros_ec_command format.
> + */
> +static int prepare_cros_ec_rx(struct cros_ec_device *ec_dev,
> + const struct cros_ish_in_msg *in_msg,
> + struct cros_ec_command *msg)
> +{
> + u8 sum = 0;
> + int i, rv, offset;
> +
> + /* Check response error code */
> + msg->result = in_msg->ec_response.result;
> + rv = cros_ec_check_result(ec_dev, msg);
> + if (rv < 0)
> + return rv;
> +
> + if (in_msg->ec_response.data_len > msg->insize) {
> + dev_err(ec_dev->dev, "Packet too long (%d bytes, expected %d)",
> + in_msg->ec_response.data_len, msg->insize);
> + return -ENOSPC;
> + }
> +
> + /* Copy response packet payload and compute checksum */
> + for (i = 0; i < sizeof(struct ec_host_response); i++)
> + sum += ((u8 *)in_msg)[IN_MSG_EC_RESPONSE_PREAMBLE + i];
> +
> + offset = sizeof(struct cros_ish_in_msg);
> + for (i = 0; i < in_msg->ec_response.data_len; i++)
> + sum += msg->data[i] = ((u8 *)in_msg)[offset + i];
> +
> + if (sum) {
> + dev_dbg(ec_dev->dev, "Bad received packet checksum %d\n", sum);
> + return -EBADMSG;
> + }
> +
> + return 0;
> +}
> +
> +static int cros_ec_pkt_xfer_ish(struct cros_ec_device *ec_dev,
> + struct cros_ec_command *msg)
> +{
> + int rv;
> + struct ishtp_cl *cros_ish_cl = ec_dev->priv;
> + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> + struct device *dev = cl_data_to_dev(client_data);
> + struct cros_ish_in_msg *in_msg = (struct cros_ish_in_msg *)ec_dev->din;
> + struct cros_ish_out_msg *out_msg =
> + (struct cros_ish_out_msg *)ec_dev->dout;
> + size_t in_size = sizeof(struct cros_ish_in_msg) + msg->insize;
> + size_t out_size = sizeof(struct cros_ish_out_msg) + msg->outsize;
> +
> + /* Sanity checks */
> + if (in_size > ec_dev->din_size) {
> + dev_err(dev,
> + "Incoming payload size %zu is too large for ec_dev->din_size %d\n",
> + in_size, ec_dev->din_size);
> + return -EMSGSIZE;
> + }
> +
> + if (out_size > ec_dev->dout_size) {
> + dev_err(dev,
> + "Outgoing payload size %zu is too large for ec_dev->dout_size %d\n",
> + out_size, ec_dev->dout_size);
> + return -EMSGSIZE;
> + }
> +
> + /* Proceed only if reset-init is not in progress */
> + if (!down_read_trylock(&init_lock)) {
> + dev_warn(dev,
> + "Host is not ready to send messages to ISH. Try again\n");
> + return -EAGAIN;
> + }
> +
> + /* Prepare the package to be sent over ISH TP */
> + out_msg->hdr.channel = CROS_EC_COMMAND;
> + out_msg->hdr.status = 0;
> +
> + ec_dev->dout += OUT_MSG_EC_REQUEST_PREAMBLE;
> + cros_ec_prepare_tx(ec_dev, msg);
> + ec_dev->dout -= OUT_MSG_EC_REQUEST_PREAMBLE;
> +
> + dev_dbg(dev,
> + "out_msg: struct_ver=0x%x checksum=0x%x command=0x%x command_ver=0x%x data_len=0x%x\n",
> + out_msg->ec_request.struct_version,
> + out_msg->ec_request.checksum,
> + out_msg->ec_request.command,
> + out_msg->ec_request.command_version,
> + out_msg->ec_request.data_len);
> +
> + /* Send command to ISH EC firmware and read response */
> + rv = ish_send(client_data,
> + (u8 *)out_msg, out_size,
> + (u8 *)in_msg, in_size);
> + if (rv < 0)
> + goto end_error;
> +
> + rv = prepare_cros_ec_rx(ec_dev, in_msg, msg);
> + if (rv)
> + goto end_error;
> +
> + rv = in_msg->ec_response.data_len;
> +
> + dev_dbg(dev,
> + "in_msg: struct_ver=0x%x checksum=0x%x result=0x%x data_len=0x%x\n",
> + in_msg->ec_response.struct_version,
> + in_msg->ec_response.checksum,
> + in_msg->ec_response.result,
> + in_msg->ec_response.data_len);
> +
> +end_error:
> + if (msg->command == EC_CMD_REBOOT_EC)
> + msleep(EC_REBOOT_DELAY_MS);
> +
> + up_read(&init_lock);
> +
> + return rv;
> +}
> +
> +static int cros_ec_dev_init(struct ishtp_cl_data *client_data)
> +{
> + struct cros_ec_device *ec_dev;
> + struct device *dev = cl_data_to_dev(client_data);
> +
> + ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
> + if (!ec_dev)
> + return -ENOMEM;
> +
> + client_data->ec_dev = ec_dev;
> + dev->driver_data = ec_dev;
> +
> + ec_dev->dev = dev;
> + ec_dev->priv = client_data->cros_ish_cl;
> + ec_dev->cmd_xfer = NULL;
> + ec_dev->pkt_xfer = cros_ec_pkt_xfer_ish;
> + ec_dev->phys_name = dev_name(dev);
> + ec_dev->din_size = sizeof(struct cros_ish_in_msg) +
> + sizeof(struct ec_response_get_protocol_info);
> + ec_dev->dout_size = sizeof(struct cros_ish_out_msg);
> +
> + return cros_ec_register(ec_dev);
> +}
> +
> +static void reset_handler(struct work_struct *work)
> +{
> + int rv;
> + struct device *dev;
> + struct ishtp_cl *cros_ish_cl;
> + struct ishtp_cl_device *cl_device;
> + struct ishtp_cl_data *client_data =
> + container_of(work, struct ishtp_cl_data, work_ishtp_reset);
> +
> + /* Lock for reset to complete */
> + down_write(&init_lock);
> +
> + cros_ish_cl = client_data->cros_ish_cl;
> + cl_device = client_data->cl_device;
> +
> + /* Unlink, flush queues & start again */
> + ishtp_cl_unlink(cros_ish_cl);
> + ishtp_cl_flush_queues(cros_ish_cl);
> + ishtp_cl_free(cros_ish_cl);
> +
> + cros_ish_cl = ishtp_cl_allocate(cl_device);
> + if (!cros_ish_cl) {
> + up_write(&init_lock);
> + return;
> + }
> +
> + ishtp_set_drvdata(cl_device, cros_ish_cl);
> + ishtp_set_client_data(cros_ish_cl, client_data);
> + client_data->cros_ish_cl = cros_ish_cl;
> +
> + rv = cros_ish_init(cros_ish_cl);
> + if (rv) {
> + ishtp_cl_free(cros_ish_cl);
> + dev_err(cl_data_to_dev(client_data), "Reset Failed\n");
> + up_write(&init_lock);
> + return;
> + }
> +
> + /* Refresh ec_dev device pointers */
> + client_data->ec_dev->priv = client_data->cros_ish_cl;
> + dev = cl_data_to_dev(client_data);
> + dev->driver_data = client_data->ec_dev;
> +
> + dev_info(cl_data_to_dev(client_data), "Chrome EC ISH reset done\n");
> +
> + up_write(&init_lock);
> +}
> +
> +/**
> + * cros_ec_ishtp_probe() - ISHTP client driver probe callback
> + * @cl_device: ISHTP client device instance
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int cros_ec_ishtp_probe(struct ishtp_cl_device *cl_device)
> +{
> + int rv;
> + struct ishtp_cl *cros_ish_cl;
> + struct ishtp_cl_data *client_data =
> + devm_kzalloc(ishtp_device(cl_device),
> + sizeof(*client_data), GFP_KERNEL);
> + if (!client_data)
> + return -ENOMEM;
> +
> + /* Lock for initialization to complete */
> + down_write(&init_lock);
> +
> + cros_ish_cl = ishtp_cl_allocate(cl_device);
> + if (!cros_ish_cl) {
> + rv = -ENOMEM;
> + goto end_ishtp_cl_alloc_error;
> + }
> +
> + ishtp_set_drvdata(cl_device, cros_ish_cl);
> + ishtp_set_client_data(cros_ish_cl, client_data);
> + client_data->cros_ish_cl = cros_ish_cl;
> + client_data->cl_device = cl_device;
> +
> + init_waitqueue_head(&client_data->response.wait_queue);
> +
> + INIT_WORK(&client_data->work_ishtp_reset,
> + reset_handler);
> + INIT_WORK(&client_data->work_ec_evt,
> + ish_evt_handler);
> +
> + rv = cros_ish_init(cros_ish_cl);
> + if (rv)
> + goto end_ishtp_cl_init_error;
> +
> + ishtp_get_device(cl_device);
> +
> + up_write(&init_lock);
> +
> + /* Register croc_ec_dev mfd */
> + rv = cros_ec_dev_init(client_data);
> + if (rv)
> + goto end_cros_ec_dev_init_error;
> +
> + return 0;
> +
> +end_cros_ec_dev_init_error:
> + ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_DISCONNECTING);
> + ishtp_cl_disconnect(cros_ish_cl);
> + ishtp_cl_unlink(cros_ish_cl);
> + ishtp_cl_flush_queues(cros_ish_cl);
> + ishtp_put_device(cl_device);
> +end_ishtp_cl_init_error:
> + ishtp_cl_free(cros_ish_cl);
> +end_ishtp_cl_alloc_error:
> + up_write(&init_lock);
> + return rv;
> +}
> +
> +/**
> + * cros_ec_ishtp_remove() - ISHTP client driver remove callback
> + * @cl_device: ISHTP client device instance
> + *
> + * Return: 0
> + */
> +static int cros_ec_ishtp_remove(struct ishtp_cl_device *cl_device)
> +{
> + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> +
> + cancel_work_sync(&client_data->work_ishtp_reset);
> + cancel_work_sync(&client_data->work_ec_evt);
> + cros_ish_deinit(cros_ish_cl);
> + ishtp_put_device(cl_device);
> +
> + return 0;
> +}
> +
> +/**
> + * cros_ec_ishtp_reset() - ISHTP client driver reset callback
> + * @cl_device: ISHTP client device instance
> + *
> + * Return: 0
> + */
> +static int cros_ec_ishtp_reset(struct ishtp_cl_device *cl_device)
> +{
> + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> +
> + schedule_work(&client_data->work_ishtp_reset);
> +
> + return 0;
> +}
> +
> +/**
> + * cros_ec_ishtp_suspend() - ISHTP client driver suspend callback
> + * @device: device instance
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int __maybe_unused cros_ec_ishtp_suspend(struct device *device)
> +{
> + struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
> + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> +
> + return cros_ec_suspend(client_data->ec_dev);
> +}
> +
> +/**
> + * cros_ec_ishtp_resume() - ISHTP client driver resume callback
> + * @device: device instance
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int __maybe_unused cros_ec_ishtp_resume(struct device *device)
> +{
> + struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
> + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> +
> + return cros_ec_resume(client_data->ec_dev);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(cros_ec_ishtp_pm_ops, cros_ec_ishtp_suspend,
> + cros_ec_ishtp_resume);
> +
> +static struct ishtp_cl_driver cros_ec_ishtp_driver = {
> + .name = "cros_ec_ishtp",
> + .guid = &cros_ish_guid,
> + .probe = cros_ec_ishtp_probe,
> + .remove = cros_ec_ishtp_remove,
> + .reset = cros_ec_ishtp_reset,
> + .driver = {
> + .pm = &cros_ec_ishtp_pm_ops,
> + },
> +};
> +
> +static int __init cros_ec_ishtp_mod_init(void)
> +{
> + return ishtp_cl_driver_register(&cros_ec_ishtp_driver, THIS_MODULE);
> +}
> +
> +static void __exit cros_ec_ishtp_mod_exit(void)
> +{
> + ishtp_cl_driver_unregister(&cros_ec_ishtp_driver);
> +}
> +
> +module_init(cros_ec_ishtp_mod_init);
> +module_exit(cros_ec_ishtp_mod_exit);
> +
> +MODULE_DESCRIPTION("ChromeOS EC ISHTP Client Driver");
> +MODULE_AUTHOR("Rushikesh S Kadam <rushikesh.s.kadam@intel.com>");
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("ishtp:*");
> --
> 1.9.1
>
^ permalink raw reply
* Re: [PATCH v5] platform: chrome: Add ChromeOS EC ISHTP driver
From: Enric Balletbo Serra @ 2019-05-15 21:23 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Rushikesh S Kadam, benjamin.tissoires, jikos, Benson Leung,
Guenter Roeck, Srinivas Pandruvada, linux-kernel, linux-input,
Nick Crews, Jett Rink, Gwendal Grignou
In-Reply-To: <ce1c6b1e-7a08-057e-898a-2ed506619cc2@collabora.com>
Missatge de Enric Balletbo i Serra <enric.balletbo@collabora.com> del
dia dc., 15 de maig 2019 a les 15:00:
>
> Hi,
>
> On 4/5/19 15:34, Rushikesh S Kadam wrote:
> > This driver implements a slim layer to enable the ChromeOS
> > EC kernel stack (cros_ec) to communicate with ChromeOS EC
> > firmware running on the Intel Integrated Sensor Hub (ISH).
> >
> > The driver registers a ChromeOS EC MFD device to connect
> > with cros_ec kernel stack (upper layer), and it registers a
> > client with the ISH Transport Protocol bus (lower layer) to
> > talk with the ISH firwmare. See description of the ISHTP
> > protocol at Documentation/hid/intel-ish-hid.txt
> >
> > Signed-off-by: Rushikesh S Kadam <rushikesh.s.kadam@intel.com>
> > Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> > Reviewed-by: Jett Rink <jettrink@chromium.org>
> > Tested-by: Jett Rink <jettrink@chromium.org>
> > ---
>
> The following patch is queued to the for-next branch for the autobuilders to
> play with, if all goes well I'll add the patch for 5.3 when current merge window
> closes.
>
Actually, I reverted this patch and applied v6.
> Thanks,
> Enric
>
> >
> > Submitting the patch to linux-input@ per the discussion here
> > https://lkml.org/lkml/2019/5/2/339
> >
> > The patch is baselined to hid git tree, branch for-5.2/ish
> > https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-5.2/ish
> >
> > v5
> > - Submitting with all Acked-by & Tested-bys. No other changes.
> >
> > v4
> > - Coding style related changes. No functional changes. Addresses
> > review comments on v3.
> >
> > v3
> > - Made several changes to improve code readability. Replaced
> > multiple cl_data_to_dev(client_data) with dev variable. Use
> > reverse Xmas tree for variable defintion where it made sense.
> > Dropped few debug prints. Add docstring for function
> > prepare_cros_ec_rx().
> > - Fix code in function prepare_cros_ec_rx() under label
> > end_cros_ec_dev_init_error.
> > - Recycle buffer in process_recv() on failing to obtain the
> > semaphore.
> > - Increase ISHTP TX/RX ring buffer size to 8.
> > - Alphabetically ordered CROS_EC_ISHTP entries in Makefile and
> > Kconfig.
> > - Updated commit message.
> >
> > v2
> > - Dropped unused "reset" parameter in function cros_ec_init()
> > - Change driver name to cros_ec_ishtp to be consistent with other
> > references in the code.
> > - Fixed a few typos.
> >
> > v1
> > - Initial version
> >
> > drivers/platform/chrome/Kconfig | 13 +
> > drivers/platform/chrome/Makefile | 1 +
> > drivers/platform/chrome/cros_ec_ishtp.c | 763 ++++++++++++++++++++++++++++++++
> > 3 files changed, 777 insertions(+)
> > create mode 100644 drivers/platform/chrome/cros_ec_ishtp.c
> >
> > diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> > index 16b1615..5848179 100644
> > --- a/drivers/platform/chrome/Kconfig
> > +++ b/drivers/platform/chrome/Kconfig
> > @@ -62,6 +62,19 @@ config CROS_EC_I2C
> > a checksum. Failing accesses will be retried three times to
> > improve reliability.
> >
> > +config CROS_EC_ISHTP
> > + tristate "ChromeOS Embedded Controller (ISHTP)"
> > + depends on MFD_CROS_EC
> > + depends on INTEL_ISH_HID
> > + help
> > + If you say Y here, you get support for talking to the ChromeOS EC
> > + firmware running on Intel Integrated Sensor Hub (ISH), using the
> > + ISH Transport protocol (ISH-TP). This uses a simple byte-level
> > + protocol with a checksum.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called cros_ec_ishtp.
> > +
> > config CROS_EC_SPI
> > tristate "ChromeOS Embedded Controller (SPI)"
> > depends on MFD_CROS_EC && SPI
> > diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> > index cd591bf..4efe102 100644
> > --- a/drivers/platform/chrome/Makefile
> > +++ b/drivers/platform/chrome/Makefile
> > @@ -7,6 +7,7 @@ cros_ec_ctl-objs := cros_ec_sysfs.o cros_ec_lightbar.o \
> > cros_ec_vbc.o cros_ec_debugfs.o
> > obj-$(CONFIG_CROS_EC_CTL) += cros_ec_ctl.o
> > obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o
> > +obj-$(CONFIG_CROS_EC_ISHTP) += cros_ec_ishtp.o
> > obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o
> > cros_ec_lpcs-objs := cros_ec_lpc.o cros_ec_lpc_reg.o
> > cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o
> > diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
> > new file mode 100644
> > index 0000000..997503d
> > --- /dev/null
> > +++ b/drivers/platform/chrome/cros_ec_ishtp.c
> > @@ -0,0 +1,763 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +// ISHTP interface for ChromeOS Embedded Controller
> > +//
> > +// Copyright (c) 2019, Intel Corporation.
> > +//
> > +// ISHTP client driver for talking to the Chrome OS EC firmware running
> > +// on Intel Integrated Sensor Hub (ISH) using the ISH Transport protocol
> > +// (ISH-TP).
> > +
> > +#include <linux/delay.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/mfd/cros_ec.h>
> > +#include <linux/mfd/cros_ec_commands.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/intel-ish-client-if.h>
> > +
> > +/*
> > + * ISH TX/RX ring buffer pool size
> > + *
> > + * The AP->ISH messages and corresponding ISH->AP responses are
> > + * serialized. We need 1 TX and 1 RX buffer for these.
> > + *
> > + * The MKBP ISH->AP events are serialized. We need one additional RX
> > + * buffer for them.
> > + */
> > +#define CROS_ISH_CL_TX_RING_SIZE 8
> > +#define CROS_ISH_CL_RX_RING_SIZE 8
> > +
> > +/* ISH CrOS EC Host Commands */
> > +enum cros_ec_ish_channel {
> > + CROS_EC_COMMAND = 1, /* AP->ISH message */
> > + CROS_MKBP_EVENT = 2, /* ISH->AP events */
> > +};
> > +
> > +/*
> > + * ISH firmware timeout for 1 message send failure is 1Hz, and the
> > + * firmware will retry 2 times, so 3Hz is used for timeout.
> > + */
> > +#define ISHTP_SEND_TIMEOUT (3 * HZ)
> > +
> > +/* ISH Transport CrOS EC ISH client unique GUID */
> > +static const guid_t cros_ish_guid =
> > + GUID_INIT(0x7b7154d0, 0x56f4, 0x4bdc,
> > + 0xb0, 0xd8, 0x9e, 0x7c, 0xda, 0xe0, 0xd6, 0xa0);
> > +
> > +struct header {
> > + u8 channel;
> > + u8 status;
> > + u8 reserved[2];
> > +} __packed;
> > +
> > +struct cros_ish_out_msg {
> > + struct header hdr;
> > + struct ec_host_request ec_request;
> > +} __packed;
> > +
> > +struct cros_ish_in_msg {
> > + struct header hdr;
> > + struct ec_host_response ec_response;
> > +} __packed;
> > +
> > +#define IN_MSG_EC_RESPONSE_PREAMBLE \
> > + offsetof(struct cros_ish_in_msg, ec_response)
> > +
> > +#define OUT_MSG_EC_REQUEST_PREAMBLE \
> > + offsetof(struct cros_ish_out_msg, ec_request)
> > +
> > +#define cl_data_to_dev(client_data) ishtp_device((client_data)->cl_device)
> > +
> > +/*
> > + * The Read-Write Semaphore is used to prevent message TX or RX while
> > + * the ishtp client is being initialized or undergoing reset.
> > + *
> > + * The readers are the kernel function calls responsible for IA->ISH
> > + * and ISH->AP messaging.
> > + *
> > + * The writers are .reset() and .probe() function.
> > + */
> > +DECLARE_RWSEM(init_lock);
> > +
> > +/**
> > + * struct response_info - Encapsulate firmware response related
> > + * information for passing between function ish_send() and
> > + * process_recv() callback.
> > + *
> > + * @data: Copy the data received from firmware here.
> > + * @max_size: Max size allocated for the @data buffer. If the received
> > + * data exceeds this value, we log an error.
> > + * @size: Actual size of data received from firmware.
> > + * @error: 0 for success, negative error code for a failure in process_recv().
> > + * @received: Set to true on receiving a valid firmware response to host command
> > + * @wait_queue: Wait queue for host to wait for firmware response.
> > + */
> > +struct response_info {
> > + void *data;
> > + size_t max_size;
> > + size_t size;
> > + int error;
> > + bool received;
> > + wait_queue_head_t wait_queue;
> > +};
> > +
> > +/**
> > + * struct ishtp_cl_data - Encapsulate per ISH TP Client.
> > + *
> > + * @cros_ish_cl: ISHTP firmware client instance.
> > + * @cl_device: ISHTP client device instance.
> > + * @response: Response info passing between ish_send() and process_recv().
> > + * @work_ishtp_reset: Work queue reset handling.
> > + * @work_ec_evt: Work queue for EC events.
> > + * @ec_dev: CrOS EC MFD device.
> > + *
> > + * This structure is used to store per client data.
> > + */
> > +struct ishtp_cl_data {
> > + struct ishtp_cl *cros_ish_cl;
> > + struct ishtp_cl_device *cl_device;
> > +
> > + /*
> > + * Used for passing firmware response information between
> > + * ish_send() and process_recv() callback.
> > + */
> > + struct response_info response;
> > +
> > + struct work_struct work_ishtp_reset;
> > + struct work_struct work_ec_evt;
> > + struct cros_ec_device *ec_dev;
> > +};
> > +
> > +/**
> > + * ish_evt_handler - ISH to AP event handler
> > + * @work: Work struct
> > + */
> > +static void ish_evt_handler(struct work_struct *work)
> > +{
> > + struct ishtp_cl_data *client_data =
> > + container_of(work, struct ishtp_cl_data, work_ec_evt);
> > + struct cros_ec_device *ec_dev = client_data->ec_dev;
> > +
> > + if (cros_ec_get_next_event(ec_dev, NULL) > 0) {
> > + blocking_notifier_call_chain(&ec_dev->event_notifier,
> > + 0, ec_dev);
> > + }
> > +}
> > +
> > +/**
> > + * ish_send() - Send message from host to firmware
> > + *
> > + * @client_data: Client data instance
> > + * @out_msg: Message buffer to be sent to firmware
> > + * @out_size: Size of out going message
> > + * @in_msg: Message buffer where the incoming data is copied. This buffer
> > + * is allocated by calling
> > + * @in_size: Max size of incoming message
> > + *
> > + * Return: Number of bytes copied in the in_msg on success, negative
> > + * error code on failure.
> > + */
> > +static int ish_send(struct ishtp_cl_data *client_data,
> > + u8 *out_msg, size_t out_size,
> > + u8 *in_msg, size_t in_size)
> > +{
> > + int rv;
> > + struct header *out_hdr = (struct header *)out_msg;
> > + struct ishtp_cl *cros_ish_cl = client_data->cros_ish_cl;
> > +
> > + dev_dbg(cl_data_to_dev(client_data),
> > + "%s: channel=%02u status=%02u\n",
> > + __func__, out_hdr->channel, out_hdr->status);
> > +
> > + /* Setup for incoming response */
> > + client_data->response.data = in_msg;
> > + client_data->response.max_size = in_size;
> > + client_data->response.error = 0;
> > + client_data->response.received = false;
> > +
> > + rv = ishtp_cl_send(cros_ish_cl, out_msg, out_size);
> > + if (rv) {
> > + dev_err(cl_data_to_dev(client_data),
> > + "ishtp_cl_send error %d\n", rv);
> > + return rv;
> > + }
> > +
> > + wait_event_interruptible_timeout(client_data->response.wait_queue,
> > + client_data->response.received,
> > + ISHTP_SEND_TIMEOUT);
> > + if (!client_data->response.received) {
> > + dev_err(cl_data_to_dev(client_data),
> > + "Timed out for response to host message\n");
> > + return -ETIMEDOUT;
> > + }
> > +
> > + if (client_data->response.error < 0)
> > + return client_data->response.error;
> > +
> > + return client_data->response.size;
> > +}
> > +
> > +/**
> > + * process_recv() - Received and parse incoming packet
> > + * @cros_ish_cl: Client instance to get stats
> > + * @rb_in_proc: Host interface message buffer
> > + *
> > + * Parse the incoming packet. If it is a response packet then it will
> > + * update per instance flags and wake up the caller waiting to for the
> > + * response. If it is an event packet then it will schedule event work.
> > + */
> > +static void process_recv(struct ishtp_cl *cros_ish_cl,
> > + struct ishtp_cl_rb *rb_in_proc)
> > +{
> > + size_t data_len = rb_in_proc->buf_idx;
> > + struct ishtp_cl_data *client_data =
> > + ishtp_get_client_data(cros_ish_cl);
> > + struct device *dev = cl_data_to_dev(client_data);
> > + struct cros_ish_in_msg *in_msg =
> > + (struct cros_ish_in_msg *)rb_in_proc->buffer.data;
> > +
> > + /* Proceed only if reset or init is not in progress */
> > + if (!down_read_trylock(&init_lock)) {
> > + /* Free the buffer */
> > + ishtp_cl_io_rb_recycle(rb_in_proc);
> > + dev_warn(dev,
> > + "Host is not ready to receive incoming messages\n");
> > + return;
> > + }
> > +
> > + /*
> > + * All firmware messages contain a header. Check the buffer size
> > + * before accessing elements inside.
> > + */
> > + if (!rb_in_proc->buffer.data) {
> > + dev_warn(dev, "rb_in_proc->buffer.data returned null");
> > + client_data->response.error = -EBADMSG;
> > + goto end_error;
> > + }
> > +
> > + if (data_len < sizeof(struct header)) {
> > + dev_err(dev, "data size %zu is less than header %zu\n",
> > + data_len, sizeof(struct header));
> > + client_data->response.error = -EMSGSIZE;
> > + goto end_error;
> > + }
> > +
> > + dev_dbg(dev, "channel=%02u status=%02u\n",
> > + in_msg->hdr.channel, in_msg->hdr.status);
> > +
> > + switch (in_msg->hdr.channel) {
> > + case CROS_EC_COMMAND:
> > + /* Sanity check */
> > + if (!client_data->response.data) {
> > + dev_err(dev,
> > + "Receiving buffer is null. Should be allocated by calling function\n");
> > + client_data->response.error = -EINVAL;
> > + goto error_wake_up;
> > + }
> > +
> > + if (client_data->response.received) {
> > + dev_err(dev,
> > + "Previous firmware message not yet processed\n");
> > + client_data->response.error = -EINVAL;
> > + goto error_wake_up;
> > + }
> > +
> > + if (data_len > client_data->response.max_size) {
> > + dev_err(dev,
> > + "Received buffer size %zu is larger than allocated buffer %zu\n",
> > + data_len, client_data->response.max_size);
> > + client_data->response.error = -EMSGSIZE;
> > + goto error_wake_up;
> > + }
> > +
> > + if (in_msg->hdr.status) {
> > + dev_err(dev, "firmware returned status %d\n",
> > + in_msg->hdr.status);
> > + client_data->response.error = -EIO;
> > + goto error_wake_up;
> > + }
> > +
> > + /* Update the actual received buffer size */
> > + client_data->response.size = data_len;
> > +
> > + /*
> > + * Copy the buffer received in firmware response for the
> > + * calling thread.
> > + */
> > + memcpy(client_data->response.data,
> > + rb_in_proc->buffer.data, data_len);
> > +
> > + /* Set flag before waking up the caller */
> > + client_data->response.received = true;
> > +error_wake_up:
> > + /* Wake the calling thread */
> > + wake_up_interruptible(&client_data->response.wait_queue);
> > +
> > + break;
> > +
> > + case CROS_MKBP_EVENT:
> > + /* The event system doesn't send any data in buffer */
> > + schedule_work(&client_data->work_ec_evt);
> > +
> > + break;
> > +
> > + default:
> > + dev_err(dev, "Invalid channel=%02d\n", in_msg->hdr.channel);
> > + }
> > +
> > +end_error:
> > + /* Free the buffer */
> > + ishtp_cl_io_rb_recycle(rb_in_proc);
> > +
> > + up_read(&init_lock);
> > +}
> > +
> > +/**
> > + * ish_event_cb() - bus driver callback for incoming message
> > + * @cl_device: ISHTP client device for which this message is targeted.
> > + *
> > + * Remove the packet from the list and process the message by calling
> > + * process_recv.
> > + */
> > +static void ish_event_cb(struct ishtp_cl_device *cl_device)
> > +{
> > + struct ishtp_cl_rb *rb_in_proc;
> > + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> > +
> > + while ((rb_in_proc = ishtp_cl_rx_get_rb(cros_ish_cl)) != NULL) {
> > + /* Decide what to do with received data */
> > + process_recv(cros_ish_cl, rb_in_proc);
> > + }
> > +}
> > +
> > +/**
> > + * cros_ish_init() - Init function for ISHTP client
> > + * @cros_ish_cl: ISHTP client instance
> > + *
> > + * This function complete the initializtion of the client.
> > + *
> > + * Return: 0 for success, negative error code for failure.
> > + */
> > +static int cros_ish_init(struct ishtp_cl *cros_ish_cl)
> > +{
> > + int rv;
> > + struct ishtp_device *dev;
> > + struct ishtp_fw_client *fw_client;
> > + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> > +
> > + rv = ishtp_cl_link(cros_ish_cl);
> > + if (rv) {
> > + dev_err(cl_data_to_dev(client_data),
> > + "ishtp_cl_link failed\n");
> > + return rv;
> > + }
> > +
> > + dev = ishtp_get_ishtp_device(cros_ish_cl);
> > +
> > + /* Connect to firmware client */
> > + ishtp_set_tx_ring_size(cros_ish_cl, CROS_ISH_CL_TX_RING_SIZE);
> > + ishtp_set_rx_ring_size(cros_ish_cl, CROS_ISH_CL_RX_RING_SIZE);
> > +
> > + fw_client = ishtp_fw_cl_get_client(dev, &cros_ish_guid);
> > + if (!fw_client) {
> > + dev_err(cl_data_to_dev(client_data),
> > + "ish client uuid not found\n");
> > + rv = -ENOENT;
> > + goto err_cl_unlink;
> > + }
> > +
> > + ishtp_cl_set_fw_client_id(cros_ish_cl,
> > + ishtp_get_fw_client_id(fw_client));
> > + ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_CONNECTING);
> > +
> > + rv = ishtp_cl_connect(cros_ish_cl);
> > + if (rv) {
> > + dev_err(cl_data_to_dev(client_data),
> > + "client connect fail\n");
> > + goto err_cl_unlink;
> > + }
> > +
> > + ishtp_register_event_cb(client_data->cl_device, ish_event_cb);
> > + return 0;
> > +
> > +err_cl_unlink:
> > + ishtp_cl_unlink(cros_ish_cl);
> > + return rv;
> > +}
> > +
> > +/**
> > + * cros_ish_deinit() - Deinit function for ISHTP client
> > + * @cros_ish_cl: ISHTP client instance
> > + *
> > + * Unlink and free cros_ec client
> > + */
> > +static void cros_ish_deinit(struct ishtp_cl *cros_ish_cl)
> > +{
> > + ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_DISCONNECTING);
> > + ishtp_cl_disconnect(cros_ish_cl);
> > + ishtp_cl_unlink(cros_ish_cl);
> > + ishtp_cl_flush_queues(cros_ish_cl);
> > +
> > + /* Disband and free all Tx and Rx client-level rings */
> > + ishtp_cl_free(cros_ish_cl);
> > +}
> > +
> > +/**
> > + * prepare_cros_ec_rx() - Check & prepare receive buffer
> > + * @ec_dev: CrOS EC MFD device.
> > + * @in_msg: Incoming message buffer
> > + * @msg: cros_ec command used to send & receive data
> > + *
> > + * Return: 0 for success, negative error code for failure.
> > + *
> > + * Check the received buffer. Convert to cros_ec_command format.
> > + */
> > +static int prepare_cros_ec_rx(struct cros_ec_device *ec_dev,
> > + const struct cros_ish_in_msg *in_msg,
> > + struct cros_ec_command *msg)
> > +{
> > + u8 sum = 0;
> > + int i, rv, offset;
> > +
> > + /* Check response error code */
> > + msg->result = in_msg->ec_response.result;
> > + rv = cros_ec_check_result(ec_dev, msg);
> > + if (rv < 0)
> > + return rv;
> > +
> > + if (in_msg->ec_response.data_len > msg->insize) {
> > + dev_err(ec_dev->dev, "Packet too long (%d bytes, expected %d)",
> > + in_msg->ec_response.data_len, msg->insize);
> > + return -ENOSPC;
> > + }
> > +
> > + /* Copy response packet payload and compute checksum */
> > + for (i = 0; i < sizeof(struct ec_host_response); i++)
> > + sum += ((u8 *)in_msg)[IN_MSG_EC_RESPONSE_PREAMBLE + i];
> > +
> > + offset = sizeof(struct cros_ish_in_msg);
> > + for (i = 0; i < in_msg->ec_response.data_len; i++)
> > + sum += msg->data[i] = ((u8 *)in_msg)[offset + i];
> > +
> > + if (sum) {
> > + dev_dbg(ec_dev->dev, "Bad received packet checksum %d\n", sum);
> > + return -EBADMSG;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int cros_ec_pkt_xfer_ish(struct cros_ec_device *ec_dev,
> > + struct cros_ec_command *msg)
> > +{
> > + int rv;
> > + struct ishtp_cl *cros_ish_cl = ec_dev->priv;
> > + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> > + struct device *dev = cl_data_to_dev(client_data);
> > + struct cros_ish_in_msg *in_msg = (struct cros_ish_in_msg *)ec_dev->din;
> > + struct cros_ish_out_msg *out_msg =
> > + (struct cros_ish_out_msg *)ec_dev->dout;
> > + size_t in_size = sizeof(struct cros_ish_in_msg) + msg->insize;
> > + size_t out_size = sizeof(struct cros_ish_out_msg) + msg->outsize;
> > +
> > + /* Proceed only if reset-init is not in progress */
> > + if (!down_read_trylock(&init_lock)) {
> > + dev_warn(dev,
> > + "Host is not ready to send messages to ISH. Try again\n");
> > + return -EAGAIN;
> > + }
> > +
> > + /* Sanity checks */
> > + if (in_size > ec_dev->din_size) {
> > + dev_err(dev,
> > + "Incoming payload size %zu is too large for ec_dev->din_size %d\n",
> > + in_size, ec_dev->din_size);
> > + return -EMSGSIZE;
> > + }
> > +
> > + if (out_size > ec_dev->dout_size) {
> > + dev_err(dev,
> > + "Outgoing payload size %zu is too large for ec_dev->dout_size %d\n",
> > + out_size, ec_dev->dout_size);
> > + return -EMSGSIZE;
> > + }
> > +
> > + /* Prepare the package to be sent over ISH TP */
> > + out_msg->hdr.channel = CROS_EC_COMMAND;
> > + out_msg->hdr.status = 0;
> > +
> > + ec_dev->dout += OUT_MSG_EC_REQUEST_PREAMBLE;
> > + cros_ec_prepare_tx(ec_dev, msg);
> > + ec_dev->dout -= OUT_MSG_EC_REQUEST_PREAMBLE;
> > +
> > + dev_dbg(dev,
> > + "out_msg: struct_ver=0x%x checksum=0x%x command=0x%x command_ver=0x%x data_len=0x%x\n",
> > + out_msg->ec_request.struct_version,
> > + out_msg->ec_request.checksum,
> > + out_msg->ec_request.command,
> > + out_msg->ec_request.command_version,
> > + out_msg->ec_request.data_len);
> > +
> > + /* Send command to ISH EC firmware and read response */
> > + rv = ish_send(client_data,
> > + (u8 *)out_msg, out_size,
> > + (u8 *)in_msg, in_size);
> > + if (rv < 0)
> > + goto end_error;
> > +
> > + rv = prepare_cros_ec_rx(ec_dev, in_msg, msg);
> > + if (rv)
> > + goto end_error;
> > +
> > + rv = in_msg->ec_response.data_len;
> > +
> > + dev_dbg(dev,
> > + "in_msg: struct_ver=0x%x checksum=0x%x result=0x%x data_len=0x%x\n",
> > + in_msg->ec_response.struct_version,
> > + in_msg->ec_response.checksum,
> > + in_msg->ec_response.result,
> > + in_msg->ec_response.data_len);
> > +
> > +end_error:
> > + if (msg->command == EC_CMD_REBOOT_EC)
> > + msleep(EC_REBOOT_DELAY_MS);
> > +
> > + up_read(&init_lock);
> > +
> > + return rv;
> > +}
> > +
> > +static int cros_ec_dev_init(struct ishtp_cl_data *client_data)
> > +{
> > + struct cros_ec_device *ec_dev;
> > + struct device *dev = cl_data_to_dev(client_data);
> > +
> > + ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
> > + if (!ec_dev)
> > + return -ENOMEM;
> > +
> > + client_data->ec_dev = ec_dev;
> > + dev->driver_data = ec_dev;
> > +
> > + ec_dev->dev = dev;
> > + ec_dev->priv = client_data->cros_ish_cl;
> > + ec_dev->cmd_xfer = NULL;
> > + ec_dev->pkt_xfer = cros_ec_pkt_xfer_ish;
> > + ec_dev->phys_name = dev_name(dev);
> > + ec_dev->din_size = sizeof(struct cros_ish_in_msg) +
> > + sizeof(struct ec_response_get_protocol_info);
> > + ec_dev->dout_size = sizeof(struct cros_ish_out_msg);
> > +
> > + return cros_ec_register(ec_dev);
> > +}
> > +
> > +static void reset_handler(struct work_struct *work)
> > +{
> > + int rv;
> > + struct device *dev;
> > + struct ishtp_cl *cros_ish_cl;
> > + struct ishtp_cl_device *cl_device;
> > + struct ishtp_cl_data *client_data =
> > + container_of(work, struct ishtp_cl_data, work_ishtp_reset);
> > +
> > + /* Lock for reset to complete */
> > + down_write(&init_lock);
> > +
> > + cros_ish_cl = client_data->cros_ish_cl;
> > + cl_device = client_data->cl_device;
> > +
> > + /* Unlink, flush queues & start again */
> > + ishtp_cl_unlink(cros_ish_cl);
> > + ishtp_cl_flush_queues(cros_ish_cl);
> > + ishtp_cl_free(cros_ish_cl);
> > +
> > + cros_ish_cl = ishtp_cl_allocate(cl_device);
> > + if (!cros_ish_cl) {
> > + up_write(&init_lock);
> > + return;
> > + }
> > +
> > + ishtp_set_drvdata(cl_device, cros_ish_cl);
> > + ishtp_set_client_data(cros_ish_cl, client_data);
> > + client_data->cros_ish_cl = cros_ish_cl;
> > +
> > + rv = cros_ish_init(cros_ish_cl);
> > + if (rv) {
> > + ishtp_cl_free(cros_ish_cl);
> > + dev_err(cl_data_to_dev(client_data), "Reset Failed\n");
> > + up_write(&init_lock);
> > + return;
> > + }
> > +
> > + /* Refresh ec_dev device pointers */
> > + client_data->ec_dev->priv = client_data->cros_ish_cl;
> > + dev = cl_data_to_dev(client_data);
> > + dev->driver_data = client_data->ec_dev;
> > +
> > + dev_info(cl_data_to_dev(client_data), "Chrome EC ISH reset done\n");
> > +
> > + up_write(&init_lock);
> > +}
> > +
> > +/**
> > + * cros_ec_ishtp_probe() - ISHTP client driver probe callback
> > + * @cl_device: ISHTP client device instance
> > + *
> > + * Return: 0 for success, negative error code for failure.
> > + */
> > +static int cros_ec_ishtp_probe(struct ishtp_cl_device *cl_device)
> > +{
> > + int rv;
> > + struct ishtp_cl *cros_ish_cl;
> > + struct ishtp_cl_data *client_data =
> > + devm_kzalloc(ishtp_device(cl_device),
> > + sizeof(*client_data), GFP_KERNEL);
> > + if (!client_data)
> > + return -ENOMEM;
> > +
> > + /* Lock for initialization to complete */
> > + down_write(&init_lock);
> > +
> > + cros_ish_cl = ishtp_cl_allocate(cl_device);
> > + if (!cros_ish_cl) {
> > + rv = -ENOMEM;
> > + goto end_ishtp_cl_alloc_error;
> > + }
> > +
> > + ishtp_set_drvdata(cl_device, cros_ish_cl);
> > + ishtp_set_client_data(cros_ish_cl, client_data);
> > + client_data->cros_ish_cl = cros_ish_cl;
> > + client_data->cl_device = cl_device;
> > +
> > + init_waitqueue_head(&client_data->response.wait_queue);
> > +
> > + INIT_WORK(&client_data->work_ishtp_reset,
> > + reset_handler);
> > + INIT_WORK(&client_data->work_ec_evt,
> > + ish_evt_handler);
> > +
> > + rv = cros_ish_init(cros_ish_cl);
> > + if (rv)
> > + goto end_ishtp_cl_init_error;
> > +
> > + ishtp_get_device(cl_device);
> > +
> > + up_write(&init_lock);
> > +
> > + /* Register croc_ec_dev mfd */
> > + rv = cros_ec_dev_init(client_data);
> > + if (rv)
> > + goto end_cros_ec_dev_init_error;
> > +
> > + return 0;
> > +
> > +end_cros_ec_dev_init_error:
> > + ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_DISCONNECTING);
> > + ishtp_cl_disconnect(cros_ish_cl);
> > + ishtp_cl_unlink(cros_ish_cl);
> > + ishtp_cl_flush_queues(cros_ish_cl);
> > + ishtp_put_device(cl_device);
> > +end_ishtp_cl_init_error:
> > + ishtp_cl_free(cros_ish_cl);
> > +end_ishtp_cl_alloc_error:
> > + up_write(&init_lock);
> > + return rv;
> > +}
> > +
> > +/**
> > + * cros_ec_ishtp_remove() - ISHTP client driver remove callback
> > + * @cl_device: ISHTP client device instance
> > + *
> > + * Return: 0
> > + */
> > +static int cros_ec_ishtp_remove(struct ishtp_cl_device *cl_device)
> > +{
> > + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> > + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> > +
> > + cancel_work_sync(&client_data->work_ishtp_reset);
> > + cancel_work_sync(&client_data->work_ec_evt);
> > + cros_ish_deinit(cros_ish_cl);
> > + ishtp_put_device(cl_device);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * cros_ec_ishtp_reset() - ISHTP client driver reset callback
> > + * @cl_device: ISHTP client device instance
> > + *
> > + * Return: 0
> > + */
> > +static int cros_ec_ishtp_reset(struct ishtp_cl_device *cl_device)
> > +{
> > + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> > + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> > +
> > + schedule_work(&client_data->work_ishtp_reset);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * cros_ec_ishtp_suspend() - ISHTP client driver suspend callback
> > + * @device: device instance
> > + *
> > + * Return: 0 for success, negative error code for failure.
> > + */
> > +static int __maybe_unused cros_ec_ishtp_suspend(struct device *device)
> > +{
> > + struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
> > + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> > + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> > +
> > + return cros_ec_suspend(client_data->ec_dev);
> > +}
> > +
> > +/**
> > + * cros_ec_ishtp_resume() - ISHTP client driver resume callback
> > + * @device: device instance
> > + *
> > + * Return: 0 for success, negative error code for failure.
> > + */
> > +static int __maybe_unused cros_ec_ishtp_resume(struct device *device)
> > +{
> > + struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
> > + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> > + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> > +
> > + return cros_ec_resume(client_data->ec_dev);
> > +}
> > +
> > +static SIMPLE_DEV_PM_OPS(cros_ec_ishtp_pm_ops, cros_ec_ishtp_suspend,
> > + cros_ec_ishtp_resume);
> > +
> > +static struct ishtp_cl_driver cros_ec_ishtp_driver = {
> > + .name = "cros_ec_ishtp",
> > + .guid = &cros_ish_guid,
> > + .probe = cros_ec_ishtp_probe,
> > + .remove = cros_ec_ishtp_remove,
> > + .reset = cros_ec_ishtp_reset,
> > + .driver = {
> > + .pm = &cros_ec_ishtp_pm_ops,
> > + },
> > +};
> > +
> > +static int __init cros_ec_ishtp_mod_init(void)
> > +{
> > + return ishtp_cl_driver_register(&cros_ec_ishtp_driver, THIS_MODULE);
> > +}
> > +
> > +static void __exit cros_ec_ishtp_mod_exit(void)
> > +{
> > + ishtp_cl_driver_unregister(&cros_ec_ishtp_driver);
> > +}
> > +
> > +module_init(cros_ec_ishtp_mod_init);
> > +module_exit(cros_ec_ishtp_mod_exit);
> > +
> > +MODULE_DESCRIPTION("ChromeOS EC ISHTP Client Driver");
> > +MODULE_AUTHOR("Rushikesh S Kadam <rushikesh.s.kadam@intel.com>");
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_ALIAS("ishtp:*");
> >
^ permalink raw reply
* Re: [PATCH V1] elan_i2c: Increment wakeup count if wake source.
From: Ravi Chandra Sadineni @ 2019-05-15 16:17 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: 廖崇榮, Benjamin Tissoires, Abhishek Bhardwaj,
Todd Broch, lkml, linux-input@vger.kernel.org
In-Reply-To: <CAKdAkRQ_J6QWxtWpoRQnNWKcJpXox6xVDZWcWYOXkBhPSn99Rw@mail.gmail.com>
Hi Dmitry,
On Mon, May 13, 2019 at 4:29 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Hi Ravi,
>
> On Mon, May 13, 2019 at 3:06 PM Ravi Chandra Sadineni
> <ravisadineni@chromium.org> wrote:
> >
> > Notify the PM core that this dev is the wake source. This helps
> > userspace daemon tracking the wake source to identify the origin of the
> > wake.
>
> I wonder if we could do that form the i2c core instead of individual drivers?
I am sorry, I don't see a way how this could be done.
>
> >
> > Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> > ---
> > drivers/input/mouse/elan_i2c_core.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> > index f9525d6f0bfe..2c0561e20b7f 100644
> > --- a/drivers/input/mouse/elan_i2c_core.c
> > +++ b/drivers/input/mouse/elan_i2c_core.c
> > @@ -981,6 +981,8 @@ static irqreturn_t elan_isr(int irq, void *dev_id)
> > if (error)
> > goto out;
> >
> > + pm_wakeup_event(dev, 0);
> > +
> > switch (report[ETP_REPORT_ID_OFFSET]) {
> > case ETP_REPORT_ID:
> > elan_report_absolute(data, report);
> > --
> > 2.20.1
> >
>
> Thanks.
>
> --
> Dmitry
Thanks,
Ravi
^ permalink raw reply
* [PATCH v2 5/5] input: goodix - Call of_device_links_add() to create links
From: Benjamin Gaignard @ 2019-05-15 13:11 UTC (permalink / raw)
To: rafael.j.wysocki, dmitry.torokhov, robh+dt, mark.rutland, hadess,
frowand.list, m.felsch, agx, arnd
Cc: linux-input, devicetree, linux-kernel, linux-stm32, broonie,
Benjamin Gaignard
In-Reply-To: <20190515131154.18373-1-benjamin.gaignard@st.com>
Add a call to of_device_links_add() to create links with
suspend dependencies at probe time.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
drivers/input/touchscreen/goodix.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index f57d82220a88..49fd4763f17b 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -30,6 +30,7 @@
#include <linux/slab.h>
#include <linux/acpi.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <asm/unaligned.h>
struct goodix_ts_data;
@@ -812,6 +813,8 @@ static int goodix_ts_probe(struct i2c_client *client,
ts->chip = goodix_get_chip_data(ts->id);
+ of_device_links_add(&client->dev);
+
if (ts->gpiod_int && ts->gpiod_rst) {
/* update device config */
ts->cfg_name = devm_kasprintf(&client->dev, GFP_KERNEL,
--
2.15.0
^ permalink raw reply related
* [PATCH v2 4/5] Input: goodix: Document suspend-dependencies property
From: Benjamin Gaignard @ 2019-05-15 13:11 UTC (permalink / raw)
To: rafael.j.wysocki, dmitry.torokhov, robh+dt, mark.rutland, hadess,
frowand.list, m.felsch, agx, arnd
Cc: linux-input, devicetree, linux-kernel, linux-stm32, broonie,
Benjamin Gaignard
In-Reply-To: <20190515131154.18373-1-benjamin.gaignard@st.com>
Explain the purpose of suspend-dependencies property.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
index 8cf0b4d38a7e..5527952054d2 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
@@ -24,6 +24,8 @@ Optional properties:
- touchscreen-size-x
- touchscreen-size-y
- touchscreen-swapped-x-y
+ - suspend-dependencies : Phandle list of devices which have to be suspended
+ after goodix device and resumed before it.
The touchscreen-* properties are documented in touchscreen.txt in this
directory.
--
2.15.0
^ permalink raw reply related
* [PATCH v2 3/5] input: edt-ft5x06 - Call of_device_links_add() to create links
From: Benjamin Gaignard @ 2019-05-15 13:11 UTC (permalink / raw)
To: rafael.j.wysocki, dmitry.torokhov, robh+dt, mark.rutland, hadess,
frowand.list, m.felsch, agx, arnd
Cc: linux-input, devicetree, linux-kernel, linux-stm32, broonie,
Benjamin Gaignard
In-Reply-To: <20190515131154.18373-1-benjamin.gaignard@st.com>
Add a call to of_device_links_add() to create links with suspend
dependencies at probe time.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
drivers/input/touchscreen/edt-ft5x06.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index 702bfda7ee77..65053be10d4e 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1167,6 +1167,8 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client,
i2c_set_clientdata(client, tsdata);
+ of_device_links_add(&client->dev);
+
irq_flags = irq_get_trigger_type(client->irq);
if (irq_flags == IRQF_TRIGGER_NONE)
irq_flags = IRQF_TRIGGER_FALLING;
--
2.15.0
^ permalink raw reply related
* [PATCH v2 2/5] Input: edt-ft5x06: Document suspend-dependencies property
From: Benjamin Gaignard @ 2019-05-15 13:11 UTC (permalink / raw)
To: rafael.j.wysocki, dmitry.torokhov, robh+dt, mark.rutland, hadess,
frowand.list, m.felsch, agx, arnd
Cc: linux-input, devicetree, linux-kernel, linux-stm32, broonie,
Benjamin Gaignard
In-Reply-To: <20190515131154.18373-1-benjamin.gaignard@st.com>
Explain the purpose of suspend-dependencies property.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
index 870b8c5cce9b..81e8eb44d720 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.txt
@@ -60,6 +60,8 @@ Optional properties:
- touchscreen-inverted-x : See touchscreen.txt
- touchscreen-inverted-y : See touchscreen.txt
- touchscreen-swapped-x-y : See touchscreen.txt
+ - suspend-dependencies : Phandle list of devices which have to be suspended
+ after touchscreen device and resumed before it.
Example:
polytouch: edt-ft5x06@38 {
--
2.15.0
^ permalink raw reply related
* [PATCH v2 1/5] of/device: Add of_device_link_add function
From: Benjamin Gaignard @ 2019-05-15 13:11 UTC (permalink / raw)
To: rafael.j.wysocki, dmitry.torokhov, robh+dt, mark.rutland, hadess,
frowand.list, m.felsch, agx, arnd
Cc: linux-input, devicetree, linux-kernel, linux-stm32, broonie,
Benjamin Gaignard
In-Reply-To: <20190515131154.18373-1-benjamin.gaignard@st.com>
Use 'suspend-dependencies' property from device node to ensure that
the listed devices will suspended after it and resumed before it.
Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
version 2:
- only keep of_device_links_add() and use
DL_FLAG_PM_RUNTIME and DL_FLAG_AUTOREMOVE_CONSUMER flags to follow Rafael
advices
- reword function description
- try to use a more explicit property name
drivers/of/device.c | 37 +++++++++++++++++++++++++++++++++++++
include/linux/of_device.h | 7 +++++++
2 files changed, 44 insertions(+)
diff --git a/drivers/of/device.c b/drivers/of/device.c
index 3717f2a20d0d..44ec84eee310 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -336,3 +336,40 @@ int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env)
return 0;
}
EXPORT_SYMBOL_GPL(of_device_uevent_modalias);
+
+/**
+ * of_device_links_add - Create links between a consumer device
+ * and it listed dependencies from device tree data
+ *
+ * @consumer: consumer device
+ *
+ * Ensure that consumer's dependencies will be suspended after it
+ * and resumed before it.
+ *
+ * Returns 0 on success, < 0 on failure.
+ */
+int of_device_links_add(struct device *consumer)
+{
+ struct device_node *np;
+ struct platform_device *pdev;
+ int i = 0;
+
+ np = of_parse_phandle(consumer->of_node, "suspend-dependencies", i++);
+ while (np) {
+ pdev = of_find_device_by_node(np);
+ of_node_put(np);
+ if (!pdev)
+ return -EINVAL;
+
+ device_link_add(consumer, &pdev->dev,
+ DL_FLAG_PM_RUNTIME |
+ DL_FLAG_AUTOREMOVE_CONSUMER);
+ platform_device_put(pdev);
+
+ np = of_parse_phandle(consumer->of_node, "suspend-dependencies",
+ i++);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_device_links_add);
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 8d31e39dd564..83f24c386d51 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -41,6 +41,8 @@ extern int of_device_request_module(struct device *dev);
extern void of_device_uevent(struct device *dev, struct kobj_uevent_env *env);
extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env);
+extern int of_device_links_add(struct device *consumer);
+
static inline void of_device_node_put(struct device *dev)
{
of_node_put(dev->of_node);
@@ -91,6 +93,11 @@ static inline int of_device_uevent_modalias(struct device *dev,
return -ENODEV;
}
+static int of_device_links_add(struct device *consumer)
+{
+ return 0;
+}
+
static inline void of_device_node_put(struct device *dev) { }
static inline const struct of_device_id *__of_match_device(
--
2.15.0
^ permalink raw reply related
* [PATCH v2 0/5] Add of_device_link_add() functions
From: Benjamin Gaignard @ 2019-05-15 13:11 UTC (permalink / raw)
To: rafael.j.wysocki, dmitry.torokhov, robh+dt, mark.rutland, hadess,
frowand.list, m.felsch, agx, arnd
Cc: linux-input, devicetree, linux-kernel, linux-stm32, broonie,
Benjamin Gaignard
It may happen that we need to ensure the order of suspend/resume
calls between two devices without obvious link.
It is for example the case on some boards where both panel and touchscreen
are sharing the same reset line. In this case we need to control which
device is going in resume first to do only one reset.
An other example is make sure that GPU is suspend before composition
hardware block to avoid pending drawing requests.
To not let everybody creating relationship between devices for free
of_device_links_add() has to be called by each driver and not in the core.
version 2:
- only keep of_device_links_add() and use
DL_FLAG_PM_RUNTIME and DL_FLAG_AUTOREMOVE_CONSUMER flags to follow Rafael
advices
- reword function description
- try to use a more explicit property name
- rebase on v5.1
Benjamin Gaignard (5):
of/device: Add of_device_link_add function
Input: edt-ft5x06: Document suspend-dependencies property
input: edt-ft5x06 - Call of_device_links_add() to create links
Input: goodix: Document suspend-dependencies property
input: goodix - Call of_device_links_add() to create links
.../bindings/input/touchscreen/edt-ft5x06.txt | 2 ++
.../bindings/input/touchscreen/goodix.txt | 2 ++
drivers/input/touchscreen/edt-ft5x06.c | 2 ++
drivers/input/touchscreen/goodix.c | 3 ++
drivers/of/device.c | 37 ++++++++++++++++++++++
include/linux/of_device.h | 7 ++++
6 files changed, 53 insertions(+)
--
2.15.0
^ permalink raw reply
* Re: [PATCH v5] platform: chrome: Add ChromeOS EC ISHTP driver
From: Enric Balletbo i Serra @ 2019-05-15 12:57 UTC (permalink / raw)
To: Rushikesh S Kadam, benjamin.tissoires, jikos, bleung, groeck,
srinivas.pandruvada
Cc: linux-kernel, linux-input, ncrews, jettrink, gwendal
In-Reply-To: <1556976893-19471-1-git-send-email-rushikesh.s.kadam@intel.com>
Hi,
On 4/5/19 15:34, Rushikesh S Kadam wrote:
> This driver implements a slim layer to enable the ChromeOS
> EC kernel stack (cros_ec) to communicate with ChromeOS EC
> firmware running on the Intel Integrated Sensor Hub (ISH).
>
> The driver registers a ChromeOS EC MFD device to connect
> with cros_ec kernel stack (upper layer), and it registers a
> client with the ISH Transport Protocol bus (lower layer) to
> talk with the ISH firwmare. See description of the ISHTP
> protocol at Documentation/hid/intel-ish-hid.txt
>
> Signed-off-by: Rushikesh S Kadam <rushikesh.s.kadam@intel.com>
> Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> Reviewed-by: Jett Rink <jettrink@chromium.org>
> Tested-by: Jett Rink <jettrink@chromium.org>
> ---
The following patch is queued to the for-next branch for the autobuilders to
play with, if all goes well I'll add the patch for 5.3 when current merge window
closes.
Thanks,
Enric
>
> Submitting the patch to linux-input@ per the discussion here
> https://lkml.org/lkml/2019/5/2/339
>
> The patch is baselined to hid git tree, branch for-5.2/ish
> https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-5.2/ish
>
> v5
> - Submitting with all Acked-by & Tested-bys. No other changes.
>
> v4
> - Coding style related changes. No functional changes. Addresses
> review comments on v3.
>
> v3
> - Made several changes to improve code readability. Replaced
> multiple cl_data_to_dev(client_data) with dev variable. Use
> reverse Xmas tree for variable defintion where it made sense.
> Dropped few debug prints. Add docstring for function
> prepare_cros_ec_rx().
> - Fix code in function prepare_cros_ec_rx() under label
> end_cros_ec_dev_init_error.
> - Recycle buffer in process_recv() on failing to obtain the
> semaphore.
> - Increase ISHTP TX/RX ring buffer size to 8.
> - Alphabetically ordered CROS_EC_ISHTP entries in Makefile and
> Kconfig.
> - Updated commit message.
>
> v2
> - Dropped unused "reset" parameter in function cros_ec_init()
> - Change driver name to cros_ec_ishtp to be consistent with other
> references in the code.
> - Fixed a few typos.
>
> v1
> - Initial version
>
> drivers/platform/chrome/Kconfig | 13 +
> drivers/platform/chrome/Makefile | 1 +
> drivers/platform/chrome/cros_ec_ishtp.c | 763 ++++++++++++++++++++++++++++++++
> 3 files changed, 777 insertions(+)
> create mode 100644 drivers/platform/chrome/cros_ec_ishtp.c
>
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 16b1615..5848179 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -62,6 +62,19 @@ config CROS_EC_I2C
> a checksum. Failing accesses will be retried three times to
> improve reliability.
>
> +config CROS_EC_ISHTP
> + tristate "ChromeOS Embedded Controller (ISHTP)"
> + depends on MFD_CROS_EC
> + depends on INTEL_ISH_HID
> + help
> + If you say Y here, you get support for talking to the ChromeOS EC
> + firmware running on Intel Integrated Sensor Hub (ISH), using the
> + ISH Transport protocol (ISH-TP). This uses a simple byte-level
> + protocol with a checksum.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called cros_ec_ishtp.
> +
> config CROS_EC_SPI
> tristate "ChromeOS Embedded Controller (SPI)"
> depends on MFD_CROS_EC && SPI
> diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
> index cd591bf..4efe102 100644
> --- a/drivers/platform/chrome/Makefile
> +++ b/drivers/platform/chrome/Makefile
> @@ -7,6 +7,7 @@ cros_ec_ctl-objs := cros_ec_sysfs.o cros_ec_lightbar.o \
> cros_ec_vbc.o cros_ec_debugfs.o
> obj-$(CONFIG_CROS_EC_CTL) += cros_ec_ctl.o
> obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o
> +obj-$(CONFIG_CROS_EC_ISHTP) += cros_ec_ishtp.o
> obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o
> cros_ec_lpcs-objs := cros_ec_lpc.o cros_ec_lpc_reg.o
> cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o
> diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
> new file mode 100644
> index 0000000..997503d
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_ec_ishtp.c
> @@ -0,0 +1,763 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// ISHTP interface for ChromeOS Embedded Controller
> +//
> +// Copyright (c) 2019, Intel Corporation.
> +//
> +// ISHTP client driver for talking to the Chrome OS EC firmware running
> +// on Intel Integrated Sensor Hub (ISH) using the ISH Transport protocol
> +// (ISH-TP).
> +
> +#include <linux/delay.h>
> +#include <linux/mfd/core.h>
> +#include <linux/mfd/cros_ec.h>
> +#include <linux/mfd/cros_ec_commands.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/intel-ish-client-if.h>
> +
> +/*
> + * ISH TX/RX ring buffer pool size
> + *
> + * The AP->ISH messages and corresponding ISH->AP responses are
> + * serialized. We need 1 TX and 1 RX buffer for these.
> + *
> + * The MKBP ISH->AP events are serialized. We need one additional RX
> + * buffer for them.
> + */
> +#define CROS_ISH_CL_TX_RING_SIZE 8
> +#define CROS_ISH_CL_RX_RING_SIZE 8
> +
> +/* ISH CrOS EC Host Commands */
> +enum cros_ec_ish_channel {
> + CROS_EC_COMMAND = 1, /* AP->ISH message */
> + CROS_MKBP_EVENT = 2, /* ISH->AP events */
> +};
> +
> +/*
> + * ISH firmware timeout for 1 message send failure is 1Hz, and the
> + * firmware will retry 2 times, so 3Hz is used for timeout.
> + */
> +#define ISHTP_SEND_TIMEOUT (3 * HZ)
> +
> +/* ISH Transport CrOS EC ISH client unique GUID */
> +static const guid_t cros_ish_guid =
> + GUID_INIT(0x7b7154d0, 0x56f4, 0x4bdc,
> + 0xb0, 0xd8, 0x9e, 0x7c, 0xda, 0xe0, 0xd6, 0xa0);
> +
> +struct header {
> + u8 channel;
> + u8 status;
> + u8 reserved[2];
> +} __packed;
> +
> +struct cros_ish_out_msg {
> + struct header hdr;
> + struct ec_host_request ec_request;
> +} __packed;
> +
> +struct cros_ish_in_msg {
> + struct header hdr;
> + struct ec_host_response ec_response;
> +} __packed;
> +
> +#define IN_MSG_EC_RESPONSE_PREAMBLE \
> + offsetof(struct cros_ish_in_msg, ec_response)
> +
> +#define OUT_MSG_EC_REQUEST_PREAMBLE \
> + offsetof(struct cros_ish_out_msg, ec_request)
> +
> +#define cl_data_to_dev(client_data) ishtp_device((client_data)->cl_device)
> +
> +/*
> + * The Read-Write Semaphore is used to prevent message TX or RX while
> + * the ishtp client is being initialized or undergoing reset.
> + *
> + * The readers are the kernel function calls responsible for IA->ISH
> + * and ISH->AP messaging.
> + *
> + * The writers are .reset() and .probe() function.
> + */
> +DECLARE_RWSEM(init_lock);
> +
> +/**
> + * struct response_info - Encapsulate firmware response related
> + * information for passing between function ish_send() and
> + * process_recv() callback.
> + *
> + * @data: Copy the data received from firmware here.
> + * @max_size: Max size allocated for the @data buffer. If the received
> + * data exceeds this value, we log an error.
> + * @size: Actual size of data received from firmware.
> + * @error: 0 for success, negative error code for a failure in process_recv().
> + * @received: Set to true on receiving a valid firmware response to host command
> + * @wait_queue: Wait queue for host to wait for firmware response.
> + */
> +struct response_info {
> + void *data;
> + size_t max_size;
> + size_t size;
> + int error;
> + bool received;
> + wait_queue_head_t wait_queue;
> +};
> +
> +/**
> + * struct ishtp_cl_data - Encapsulate per ISH TP Client.
> + *
> + * @cros_ish_cl: ISHTP firmware client instance.
> + * @cl_device: ISHTP client device instance.
> + * @response: Response info passing between ish_send() and process_recv().
> + * @work_ishtp_reset: Work queue reset handling.
> + * @work_ec_evt: Work queue for EC events.
> + * @ec_dev: CrOS EC MFD device.
> + *
> + * This structure is used to store per client data.
> + */
> +struct ishtp_cl_data {
> + struct ishtp_cl *cros_ish_cl;
> + struct ishtp_cl_device *cl_device;
> +
> + /*
> + * Used for passing firmware response information between
> + * ish_send() and process_recv() callback.
> + */
> + struct response_info response;
> +
> + struct work_struct work_ishtp_reset;
> + struct work_struct work_ec_evt;
> + struct cros_ec_device *ec_dev;
> +};
> +
> +/**
> + * ish_evt_handler - ISH to AP event handler
> + * @work: Work struct
> + */
> +static void ish_evt_handler(struct work_struct *work)
> +{
> + struct ishtp_cl_data *client_data =
> + container_of(work, struct ishtp_cl_data, work_ec_evt);
> + struct cros_ec_device *ec_dev = client_data->ec_dev;
> +
> + if (cros_ec_get_next_event(ec_dev, NULL) > 0) {
> + blocking_notifier_call_chain(&ec_dev->event_notifier,
> + 0, ec_dev);
> + }
> +}
> +
> +/**
> + * ish_send() - Send message from host to firmware
> + *
> + * @client_data: Client data instance
> + * @out_msg: Message buffer to be sent to firmware
> + * @out_size: Size of out going message
> + * @in_msg: Message buffer where the incoming data is copied. This buffer
> + * is allocated by calling
> + * @in_size: Max size of incoming message
> + *
> + * Return: Number of bytes copied in the in_msg on success, negative
> + * error code on failure.
> + */
> +static int ish_send(struct ishtp_cl_data *client_data,
> + u8 *out_msg, size_t out_size,
> + u8 *in_msg, size_t in_size)
> +{
> + int rv;
> + struct header *out_hdr = (struct header *)out_msg;
> + struct ishtp_cl *cros_ish_cl = client_data->cros_ish_cl;
> +
> + dev_dbg(cl_data_to_dev(client_data),
> + "%s: channel=%02u status=%02u\n",
> + __func__, out_hdr->channel, out_hdr->status);
> +
> + /* Setup for incoming response */
> + client_data->response.data = in_msg;
> + client_data->response.max_size = in_size;
> + client_data->response.error = 0;
> + client_data->response.received = false;
> +
> + rv = ishtp_cl_send(cros_ish_cl, out_msg, out_size);
> + if (rv) {
> + dev_err(cl_data_to_dev(client_data),
> + "ishtp_cl_send error %d\n", rv);
> + return rv;
> + }
> +
> + wait_event_interruptible_timeout(client_data->response.wait_queue,
> + client_data->response.received,
> + ISHTP_SEND_TIMEOUT);
> + if (!client_data->response.received) {
> + dev_err(cl_data_to_dev(client_data),
> + "Timed out for response to host message\n");
> + return -ETIMEDOUT;
> + }
> +
> + if (client_data->response.error < 0)
> + return client_data->response.error;
> +
> + return client_data->response.size;
> +}
> +
> +/**
> + * process_recv() - Received and parse incoming packet
> + * @cros_ish_cl: Client instance to get stats
> + * @rb_in_proc: Host interface message buffer
> + *
> + * Parse the incoming packet. If it is a response packet then it will
> + * update per instance flags and wake up the caller waiting to for the
> + * response. If it is an event packet then it will schedule event work.
> + */
> +static void process_recv(struct ishtp_cl *cros_ish_cl,
> + struct ishtp_cl_rb *rb_in_proc)
> +{
> + size_t data_len = rb_in_proc->buf_idx;
> + struct ishtp_cl_data *client_data =
> + ishtp_get_client_data(cros_ish_cl);
> + struct device *dev = cl_data_to_dev(client_data);
> + struct cros_ish_in_msg *in_msg =
> + (struct cros_ish_in_msg *)rb_in_proc->buffer.data;
> +
> + /* Proceed only if reset or init is not in progress */
> + if (!down_read_trylock(&init_lock)) {
> + /* Free the buffer */
> + ishtp_cl_io_rb_recycle(rb_in_proc);
> + dev_warn(dev,
> + "Host is not ready to receive incoming messages\n");
> + return;
> + }
> +
> + /*
> + * All firmware messages contain a header. Check the buffer size
> + * before accessing elements inside.
> + */
> + if (!rb_in_proc->buffer.data) {
> + dev_warn(dev, "rb_in_proc->buffer.data returned null");
> + client_data->response.error = -EBADMSG;
> + goto end_error;
> + }
> +
> + if (data_len < sizeof(struct header)) {
> + dev_err(dev, "data size %zu is less than header %zu\n",
> + data_len, sizeof(struct header));
> + client_data->response.error = -EMSGSIZE;
> + goto end_error;
> + }
> +
> + dev_dbg(dev, "channel=%02u status=%02u\n",
> + in_msg->hdr.channel, in_msg->hdr.status);
> +
> + switch (in_msg->hdr.channel) {
> + case CROS_EC_COMMAND:
> + /* Sanity check */
> + if (!client_data->response.data) {
> + dev_err(dev,
> + "Receiving buffer is null. Should be allocated by calling function\n");
> + client_data->response.error = -EINVAL;
> + goto error_wake_up;
> + }
> +
> + if (client_data->response.received) {
> + dev_err(dev,
> + "Previous firmware message not yet processed\n");
> + client_data->response.error = -EINVAL;
> + goto error_wake_up;
> + }
> +
> + if (data_len > client_data->response.max_size) {
> + dev_err(dev,
> + "Received buffer size %zu is larger than allocated buffer %zu\n",
> + data_len, client_data->response.max_size);
> + client_data->response.error = -EMSGSIZE;
> + goto error_wake_up;
> + }
> +
> + if (in_msg->hdr.status) {
> + dev_err(dev, "firmware returned status %d\n",
> + in_msg->hdr.status);
> + client_data->response.error = -EIO;
> + goto error_wake_up;
> + }
> +
> + /* Update the actual received buffer size */
> + client_data->response.size = data_len;
> +
> + /*
> + * Copy the buffer received in firmware response for the
> + * calling thread.
> + */
> + memcpy(client_data->response.data,
> + rb_in_proc->buffer.data, data_len);
> +
> + /* Set flag before waking up the caller */
> + client_data->response.received = true;
> +error_wake_up:
> + /* Wake the calling thread */
> + wake_up_interruptible(&client_data->response.wait_queue);
> +
> + break;
> +
> + case CROS_MKBP_EVENT:
> + /* The event system doesn't send any data in buffer */
> + schedule_work(&client_data->work_ec_evt);
> +
> + break;
> +
> + default:
> + dev_err(dev, "Invalid channel=%02d\n", in_msg->hdr.channel);
> + }
> +
> +end_error:
> + /* Free the buffer */
> + ishtp_cl_io_rb_recycle(rb_in_proc);
> +
> + up_read(&init_lock);
> +}
> +
> +/**
> + * ish_event_cb() - bus driver callback for incoming message
> + * @cl_device: ISHTP client device for which this message is targeted.
> + *
> + * Remove the packet from the list and process the message by calling
> + * process_recv.
> + */
> +static void ish_event_cb(struct ishtp_cl_device *cl_device)
> +{
> + struct ishtp_cl_rb *rb_in_proc;
> + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> +
> + while ((rb_in_proc = ishtp_cl_rx_get_rb(cros_ish_cl)) != NULL) {
> + /* Decide what to do with received data */
> + process_recv(cros_ish_cl, rb_in_proc);
> + }
> +}
> +
> +/**
> + * cros_ish_init() - Init function for ISHTP client
> + * @cros_ish_cl: ISHTP client instance
> + *
> + * This function complete the initializtion of the client.
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int cros_ish_init(struct ishtp_cl *cros_ish_cl)
> +{
> + int rv;
> + struct ishtp_device *dev;
> + struct ishtp_fw_client *fw_client;
> + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> +
> + rv = ishtp_cl_link(cros_ish_cl);
> + if (rv) {
> + dev_err(cl_data_to_dev(client_data),
> + "ishtp_cl_link failed\n");
> + return rv;
> + }
> +
> + dev = ishtp_get_ishtp_device(cros_ish_cl);
> +
> + /* Connect to firmware client */
> + ishtp_set_tx_ring_size(cros_ish_cl, CROS_ISH_CL_TX_RING_SIZE);
> + ishtp_set_rx_ring_size(cros_ish_cl, CROS_ISH_CL_RX_RING_SIZE);
> +
> + fw_client = ishtp_fw_cl_get_client(dev, &cros_ish_guid);
> + if (!fw_client) {
> + dev_err(cl_data_to_dev(client_data),
> + "ish client uuid not found\n");
> + rv = -ENOENT;
> + goto err_cl_unlink;
> + }
> +
> + ishtp_cl_set_fw_client_id(cros_ish_cl,
> + ishtp_get_fw_client_id(fw_client));
> + ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_CONNECTING);
> +
> + rv = ishtp_cl_connect(cros_ish_cl);
> + if (rv) {
> + dev_err(cl_data_to_dev(client_data),
> + "client connect fail\n");
> + goto err_cl_unlink;
> + }
> +
> + ishtp_register_event_cb(client_data->cl_device, ish_event_cb);
> + return 0;
> +
> +err_cl_unlink:
> + ishtp_cl_unlink(cros_ish_cl);
> + return rv;
> +}
> +
> +/**
> + * cros_ish_deinit() - Deinit function for ISHTP client
> + * @cros_ish_cl: ISHTP client instance
> + *
> + * Unlink and free cros_ec client
> + */
> +static void cros_ish_deinit(struct ishtp_cl *cros_ish_cl)
> +{
> + ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_DISCONNECTING);
> + ishtp_cl_disconnect(cros_ish_cl);
> + ishtp_cl_unlink(cros_ish_cl);
> + ishtp_cl_flush_queues(cros_ish_cl);
> +
> + /* Disband and free all Tx and Rx client-level rings */
> + ishtp_cl_free(cros_ish_cl);
> +}
> +
> +/**
> + * prepare_cros_ec_rx() - Check & prepare receive buffer
> + * @ec_dev: CrOS EC MFD device.
> + * @in_msg: Incoming message buffer
> + * @msg: cros_ec command used to send & receive data
> + *
> + * Return: 0 for success, negative error code for failure.
> + *
> + * Check the received buffer. Convert to cros_ec_command format.
> + */
> +static int prepare_cros_ec_rx(struct cros_ec_device *ec_dev,
> + const struct cros_ish_in_msg *in_msg,
> + struct cros_ec_command *msg)
> +{
> + u8 sum = 0;
> + int i, rv, offset;
> +
> + /* Check response error code */
> + msg->result = in_msg->ec_response.result;
> + rv = cros_ec_check_result(ec_dev, msg);
> + if (rv < 0)
> + return rv;
> +
> + if (in_msg->ec_response.data_len > msg->insize) {
> + dev_err(ec_dev->dev, "Packet too long (%d bytes, expected %d)",
> + in_msg->ec_response.data_len, msg->insize);
> + return -ENOSPC;
> + }
> +
> + /* Copy response packet payload and compute checksum */
> + for (i = 0; i < sizeof(struct ec_host_response); i++)
> + sum += ((u8 *)in_msg)[IN_MSG_EC_RESPONSE_PREAMBLE + i];
> +
> + offset = sizeof(struct cros_ish_in_msg);
> + for (i = 0; i < in_msg->ec_response.data_len; i++)
> + sum += msg->data[i] = ((u8 *)in_msg)[offset + i];
> +
> + if (sum) {
> + dev_dbg(ec_dev->dev, "Bad received packet checksum %d\n", sum);
> + return -EBADMSG;
> + }
> +
> + return 0;
> +}
> +
> +static int cros_ec_pkt_xfer_ish(struct cros_ec_device *ec_dev,
> + struct cros_ec_command *msg)
> +{
> + int rv;
> + struct ishtp_cl *cros_ish_cl = ec_dev->priv;
> + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> + struct device *dev = cl_data_to_dev(client_data);
> + struct cros_ish_in_msg *in_msg = (struct cros_ish_in_msg *)ec_dev->din;
> + struct cros_ish_out_msg *out_msg =
> + (struct cros_ish_out_msg *)ec_dev->dout;
> + size_t in_size = sizeof(struct cros_ish_in_msg) + msg->insize;
> + size_t out_size = sizeof(struct cros_ish_out_msg) + msg->outsize;
> +
> + /* Proceed only if reset-init is not in progress */
> + if (!down_read_trylock(&init_lock)) {
> + dev_warn(dev,
> + "Host is not ready to send messages to ISH. Try again\n");
> + return -EAGAIN;
> + }
> +
> + /* Sanity checks */
> + if (in_size > ec_dev->din_size) {
> + dev_err(dev,
> + "Incoming payload size %zu is too large for ec_dev->din_size %d\n",
> + in_size, ec_dev->din_size);
> + return -EMSGSIZE;
> + }
> +
> + if (out_size > ec_dev->dout_size) {
> + dev_err(dev,
> + "Outgoing payload size %zu is too large for ec_dev->dout_size %d\n",
> + out_size, ec_dev->dout_size);
> + return -EMSGSIZE;
> + }
> +
> + /* Prepare the package to be sent over ISH TP */
> + out_msg->hdr.channel = CROS_EC_COMMAND;
> + out_msg->hdr.status = 0;
> +
> + ec_dev->dout += OUT_MSG_EC_REQUEST_PREAMBLE;
> + cros_ec_prepare_tx(ec_dev, msg);
> + ec_dev->dout -= OUT_MSG_EC_REQUEST_PREAMBLE;
> +
> + dev_dbg(dev,
> + "out_msg: struct_ver=0x%x checksum=0x%x command=0x%x command_ver=0x%x data_len=0x%x\n",
> + out_msg->ec_request.struct_version,
> + out_msg->ec_request.checksum,
> + out_msg->ec_request.command,
> + out_msg->ec_request.command_version,
> + out_msg->ec_request.data_len);
> +
> + /* Send command to ISH EC firmware and read response */
> + rv = ish_send(client_data,
> + (u8 *)out_msg, out_size,
> + (u8 *)in_msg, in_size);
> + if (rv < 0)
> + goto end_error;
> +
> + rv = prepare_cros_ec_rx(ec_dev, in_msg, msg);
> + if (rv)
> + goto end_error;
> +
> + rv = in_msg->ec_response.data_len;
> +
> + dev_dbg(dev,
> + "in_msg: struct_ver=0x%x checksum=0x%x result=0x%x data_len=0x%x\n",
> + in_msg->ec_response.struct_version,
> + in_msg->ec_response.checksum,
> + in_msg->ec_response.result,
> + in_msg->ec_response.data_len);
> +
> +end_error:
> + if (msg->command == EC_CMD_REBOOT_EC)
> + msleep(EC_REBOOT_DELAY_MS);
> +
> + up_read(&init_lock);
> +
> + return rv;
> +}
> +
> +static int cros_ec_dev_init(struct ishtp_cl_data *client_data)
> +{
> + struct cros_ec_device *ec_dev;
> + struct device *dev = cl_data_to_dev(client_data);
> +
> + ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
> + if (!ec_dev)
> + return -ENOMEM;
> +
> + client_data->ec_dev = ec_dev;
> + dev->driver_data = ec_dev;
> +
> + ec_dev->dev = dev;
> + ec_dev->priv = client_data->cros_ish_cl;
> + ec_dev->cmd_xfer = NULL;
> + ec_dev->pkt_xfer = cros_ec_pkt_xfer_ish;
> + ec_dev->phys_name = dev_name(dev);
> + ec_dev->din_size = sizeof(struct cros_ish_in_msg) +
> + sizeof(struct ec_response_get_protocol_info);
> + ec_dev->dout_size = sizeof(struct cros_ish_out_msg);
> +
> + return cros_ec_register(ec_dev);
> +}
> +
> +static void reset_handler(struct work_struct *work)
> +{
> + int rv;
> + struct device *dev;
> + struct ishtp_cl *cros_ish_cl;
> + struct ishtp_cl_device *cl_device;
> + struct ishtp_cl_data *client_data =
> + container_of(work, struct ishtp_cl_data, work_ishtp_reset);
> +
> + /* Lock for reset to complete */
> + down_write(&init_lock);
> +
> + cros_ish_cl = client_data->cros_ish_cl;
> + cl_device = client_data->cl_device;
> +
> + /* Unlink, flush queues & start again */
> + ishtp_cl_unlink(cros_ish_cl);
> + ishtp_cl_flush_queues(cros_ish_cl);
> + ishtp_cl_free(cros_ish_cl);
> +
> + cros_ish_cl = ishtp_cl_allocate(cl_device);
> + if (!cros_ish_cl) {
> + up_write(&init_lock);
> + return;
> + }
> +
> + ishtp_set_drvdata(cl_device, cros_ish_cl);
> + ishtp_set_client_data(cros_ish_cl, client_data);
> + client_data->cros_ish_cl = cros_ish_cl;
> +
> + rv = cros_ish_init(cros_ish_cl);
> + if (rv) {
> + ishtp_cl_free(cros_ish_cl);
> + dev_err(cl_data_to_dev(client_data), "Reset Failed\n");
> + up_write(&init_lock);
> + return;
> + }
> +
> + /* Refresh ec_dev device pointers */
> + client_data->ec_dev->priv = client_data->cros_ish_cl;
> + dev = cl_data_to_dev(client_data);
> + dev->driver_data = client_data->ec_dev;
> +
> + dev_info(cl_data_to_dev(client_data), "Chrome EC ISH reset done\n");
> +
> + up_write(&init_lock);
> +}
> +
> +/**
> + * cros_ec_ishtp_probe() - ISHTP client driver probe callback
> + * @cl_device: ISHTP client device instance
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int cros_ec_ishtp_probe(struct ishtp_cl_device *cl_device)
> +{
> + int rv;
> + struct ishtp_cl *cros_ish_cl;
> + struct ishtp_cl_data *client_data =
> + devm_kzalloc(ishtp_device(cl_device),
> + sizeof(*client_data), GFP_KERNEL);
> + if (!client_data)
> + return -ENOMEM;
> +
> + /* Lock for initialization to complete */
> + down_write(&init_lock);
> +
> + cros_ish_cl = ishtp_cl_allocate(cl_device);
> + if (!cros_ish_cl) {
> + rv = -ENOMEM;
> + goto end_ishtp_cl_alloc_error;
> + }
> +
> + ishtp_set_drvdata(cl_device, cros_ish_cl);
> + ishtp_set_client_data(cros_ish_cl, client_data);
> + client_data->cros_ish_cl = cros_ish_cl;
> + client_data->cl_device = cl_device;
> +
> + init_waitqueue_head(&client_data->response.wait_queue);
> +
> + INIT_WORK(&client_data->work_ishtp_reset,
> + reset_handler);
> + INIT_WORK(&client_data->work_ec_evt,
> + ish_evt_handler);
> +
> + rv = cros_ish_init(cros_ish_cl);
> + if (rv)
> + goto end_ishtp_cl_init_error;
> +
> + ishtp_get_device(cl_device);
> +
> + up_write(&init_lock);
> +
> + /* Register croc_ec_dev mfd */
> + rv = cros_ec_dev_init(client_data);
> + if (rv)
> + goto end_cros_ec_dev_init_error;
> +
> + return 0;
> +
> +end_cros_ec_dev_init_error:
> + ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_DISCONNECTING);
> + ishtp_cl_disconnect(cros_ish_cl);
> + ishtp_cl_unlink(cros_ish_cl);
> + ishtp_cl_flush_queues(cros_ish_cl);
> + ishtp_put_device(cl_device);
> +end_ishtp_cl_init_error:
> + ishtp_cl_free(cros_ish_cl);
> +end_ishtp_cl_alloc_error:
> + up_write(&init_lock);
> + return rv;
> +}
> +
> +/**
> + * cros_ec_ishtp_remove() - ISHTP client driver remove callback
> + * @cl_device: ISHTP client device instance
> + *
> + * Return: 0
> + */
> +static int cros_ec_ishtp_remove(struct ishtp_cl_device *cl_device)
> +{
> + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> +
> + cancel_work_sync(&client_data->work_ishtp_reset);
> + cancel_work_sync(&client_data->work_ec_evt);
> + cros_ish_deinit(cros_ish_cl);
> + ishtp_put_device(cl_device);
> +
> + return 0;
> +}
> +
> +/**
> + * cros_ec_ishtp_reset() - ISHTP client driver reset callback
> + * @cl_device: ISHTP client device instance
> + *
> + * Return: 0
> + */
> +static int cros_ec_ishtp_reset(struct ishtp_cl_device *cl_device)
> +{
> + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> +
> + schedule_work(&client_data->work_ishtp_reset);
> +
> + return 0;
> +}
> +
> +/**
> + * cros_ec_ishtp_suspend() - ISHTP client driver suspend callback
> + * @device: device instance
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int __maybe_unused cros_ec_ishtp_suspend(struct device *device)
> +{
> + struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
> + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> +
> + return cros_ec_suspend(client_data->ec_dev);
> +}
> +
> +/**
> + * cros_ec_ishtp_resume() - ISHTP client driver resume callback
> + * @device: device instance
> + *
> + * Return: 0 for success, negative error code for failure.
> + */
> +static int __maybe_unused cros_ec_ishtp_resume(struct device *device)
> +{
> + struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
> + struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
> + struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
> +
> + return cros_ec_resume(client_data->ec_dev);
> +}
> +
> +static SIMPLE_DEV_PM_OPS(cros_ec_ishtp_pm_ops, cros_ec_ishtp_suspend,
> + cros_ec_ishtp_resume);
> +
> +static struct ishtp_cl_driver cros_ec_ishtp_driver = {
> + .name = "cros_ec_ishtp",
> + .guid = &cros_ish_guid,
> + .probe = cros_ec_ishtp_probe,
> + .remove = cros_ec_ishtp_remove,
> + .reset = cros_ec_ishtp_reset,
> + .driver = {
> + .pm = &cros_ec_ishtp_pm_ops,
> + },
> +};
> +
> +static int __init cros_ec_ishtp_mod_init(void)
> +{
> + return ishtp_cl_driver_register(&cros_ec_ishtp_driver, THIS_MODULE);
> +}
> +
> +static void __exit cros_ec_ishtp_mod_exit(void)
> +{
> + ishtp_cl_driver_unregister(&cros_ec_ishtp_driver);
> +}
> +
> +module_init(cros_ec_ishtp_mod_init);
> +module_exit(cros_ec_ishtp_mod_exit);
> +
> +MODULE_DESCRIPTION("ChromeOS EC ISHTP Client Driver");
> +MODULE_AUTHOR("Rushikesh S Kadam <rushikesh.s.kadam@intel.com>");
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("ishtp:*");
>
^ permalink raw reply
* [PATCH v6] platform: chrome: Add ChromeOS EC ISHTP driver
From: Rushikesh S Kadam @ 2019-05-15 10:38 UTC (permalink / raw)
To: benjamin.tissoires, jikos, bleung, enric.balletbo, groeck,
srinivas.pandruvada
Cc: linux-kernel, linux-input, ncrews, jettrink, gwendal,
rushikesh.s.kadam
This driver implements a slim layer to enable the ChromeOS
EC kernel stack (cros_ec) to communicate with ChromeOS EC
firmware running on the Intel Integrated Sensor Hub (ISH).
The driver registers a ChromeOS EC MFD device to connect
with cros_ec kernel stack (upper layer), and it registers a
client with the ISH Transport Protocol bus (lower layer) to
talk with the ISH firwmare. See description of the ISHTP
protocol at Documentation/hid/intel-ish-hid.txt
Signed-off-by: Rushikesh S Kadam <rushikesh.s.kadam@intel.com>
Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Reviewed-by: Jett Rink <jettrink@chromium.org>
Tested-by: Jett Rink <jettrink@chromium.org>
---
Submitting the patch to linux-input@ per the discussion here
https://lkml.org/lkml/2019/5/2/339
The patch is baselined to hid git tree, branch for-5.2/ish
https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git/log/?h=for-5.2/ish
v6
- Moved the sanity checks in cros_ec_pkt_xfer_ish() to before
the point we take the lock (Bug fix).
v5
- Submitting with all Acked-by & Tested-bys. No other changes.
v4
- Coding style related changes. No functional changes. Addresses
review comments on v3.
v3
- Made several changes to improve code readability. Replaced
multiple cl_data_to_dev(client_data) with dev variable. Use
reverse Xmas tree for variable defintion where it made sense.
Dropped few debug prints. Add docstring for function
prepare_cros_ec_rx().
- Fix code in function prepare_cros_ec_rx() under label
end_cros_ec_dev_init_error.
- Recycle buffer in process_recv() on failing to obtain the
semaphore.
- Increase ISHTP TX/RX ring buffer size to 8.
- Alphabetically ordered CROS_EC_ISHTP entries in Makefile and
Kconfig.
- Updated commit message.
v2
- Dropped unused "reset" parameter in function cros_ec_init()
- Change driver name to cros_ec_ishtp to be consistent with other
references in the code.
- Fixed a few typos.
v1
- Initial version
drivers/platform/chrome/Kconfig | 13 +
drivers/platform/chrome/Makefile | 1 +
drivers/platform/chrome/cros_ec_ishtp.c | 763 ++++++++++++++++++++++++++++++++
3 files changed, 777 insertions(+)
create mode 100644 drivers/platform/chrome/cros_ec_ishtp.c
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 16b1615..5848179 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -62,6 +62,19 @@ config CROS_EC_I2C
a checksum. Failing accesses will be retried three times to
improve reliability.
+config CROS_EC_ISHTP
+ tristate "ChromeOS Embedded Controller (ISHTP)"
+ depends on MFD_CROS_EC
+ depends on INTEL_ISH_HID
+ help
+ If you say Y here, you get support for talking to the ChromeOS EC
+ firmware running on Intel Integrated Sensor Hub (ISH), using the
+ ISH Transport protocol (ISH-TP). This uses a simple byte-level
+ protocol with a checksum.
+
+ To compile this driver as a module, choose M here: the
+ module will be called cros_ec_ishtp.
+
config CROS_EC_SPI
tristate "ChromeOS Embedded Controller (SPI)"
depends on MFD_CROS_EC && SPI
diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index cd591bf..4efe102 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -7,6 +7,7 @@ cros_ec_ctl-objs := cros_ec_sysfs.o cros_ec_lightbar.o \
cros_ec_vbc.o cros_ec_debugfs.o
obj-$(CONFIG_CROS_EC_CTL) += cros_ec_ctl.o
obj-$(CONFIG_CROS_EC_I2C) += cros_ec_i2c.o
+obj-$(CONFIG_CROS_EC_ISHTP) += cros_ec_ishtp.o
obj-$(CONFIG_CROS_EC_SPI) += cros_ec_spi.o
cros_ec_lpcs-objs := cros_ec_lpc.o cros_ec_lpc_reg.o
cros_ec_lpcs-$(CONFIG_CROS_EC_LPC_MEC) += cros_ec_lpc_mec.o
diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
new file mode 100644
index 0000000..e504d25
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_ishtp.c
@@ -0,0 +1,763 @@
+// SPDX-License-Identifier: GPL-2.0
+// ISHTP interface for ChromeOS Embedded Controller
+//
+// Copyright (c) 2019, Intel Corporation.
+//
+// ISHTP client driver for talking to the Chrome OS EC firmware running
+// on Intel Integrated Sensor Hub (ISH) using the ISH Transport protocol
+// (ISH-TP).
+
+#include <linux/delay.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/intel-ish-client-if.h>
+
+/*
+ * ISH TX/RX ring buffer pool size
+ *
+ * The AP->ISH messages and corresponding ISH->AP responses are
+ * serialized. We need 1 TX and 1 RX buffer for these.
+ *
+ * The MKBP ISH->AP events are serialized. We need one additional RX
+ * buffer for them.
+ */
+#define CROS_ISH_CL_TX_RING_SIZE 8
+#define CROS_ISH_CL_RX_RING_SIZE 8
+
+/* ISH CrOS EC Host Commands */
+enum cros_ec_ish_channel {
+ CROS_EC_COMMAND = 1, /* AP->ISH message */
+ CROS_MKBP_EVENT = 2, /* ISH->AP events */
+};
+
+/*
+ * ISH firmware timeout for 1 message send failure is 1Hz, and the
+ * firmware will retry 2 times, so 3Hz is used for timeout.
+ */
+#define ISHTP_SEND_TIMEOUT (3 * HZ)
+
+/* ISH Transport CrOS EC ISH client unique GUID */
+static const guid_t cros_ish_guid =
+ GUID_INIT(0x7b7154d0, 0x56f4, 0x4bdc,
+ 0xb0, 0xd8, 0x9e, 0x7c, 0xda, 0xe0, 0xd6, 0xa0);
+
+struct header {
+ u8 channel;
+ u8 status;
+ u8 reserved[2];
+} __packed;
+
+struct cros_ish_out_msg {
+ struct header hdr;
+ struct ec_host_request ec_request;
+} __packed;
+
+struct cros_ish_in_msg {
+ struct header hdr;
+ struct ec_host_response ec_response;
+} __packed;
+
+#define IN_MSG_EC_RESPONSE_PREAMBLE \
+ offsetof(struct cros_ish_in_msg, ec_response)
+
+#define OUT_MSG_EC_REQUEST_PREAMBLE \
+ offsetof(struct cros_ish_out_msg, ec_request)
+
+#define cl_data_to_dev(client_data) ishtp_device((client_data)->cl_device)
+
+/*
+ * The Read-Write Semaphore is used to prevent message TX or RX while
+ * the ishtp client is being initialized or undergoing reset.
+ *
+ * The readers are the kernel function calls responsible for IA->ISH
+ * and ISH->AP messaging.
+ *
+ * The writers are .reset() and .probe() function.
+ */
+DECLARE_RWSEM(init_lock);
+
+/**
+ * struct response_info - Encapsulate firmware response related
+ * information for passing between function ish_send() and
+ * process_recv() callback.
+ *
+ * @data: Copy the data received from firmware here.
+ * @max_size: Max size allocated for the @data buffer. If the received
+ * data exceeds this value, we log an error.
+ * @size: Actual size of data received from firmware.
+ * @error: 0 for success, negative error code for a failure in process_recv().
+ * @received: Set to true on receiving a valid firmware response to host command
+ * @wait_queue: Wait queue for host to wait for firmware response.
+ */
+struct response_info {
+ void *data;
+ size_t max_size;
+ size_t size;
+ int error;
+ bool received;
+ wait_queue_head_t wait_queue;
+};
+
+/**
+ * struct ishtp_cl_data - Encapsulate per ISH TP Client.
+ *
+ * @cros_ish_cl: ISHTP firmware client instance.
+ * @cl_device: ISHTP client device instance.
+ * @response: Response info passing between ish_send() and process_recv().
+ * @work_ishtp_reset: Work queue reset handling.
+ * @work_ec_evt: Work queue for EC events.
+ * @ec_dev: CrOS EC MFD device.
+ *
+ * This structure is used to store per client data.
+ */
+struct ishtp_cl_data {
+ struct ishtp_cl *cros_ish_cl;
+ struct ishtp_cl_device *cl_device;
+
+ /*
+ * Used for passing firmware response information between
+ * ish_send() and process_recv() callback.
+ */
+ struct response_info response;
+
+ struct work_struct work_ishtp_reset;
+ struct work_struct work_ec_evt;
+ struct cros_ec_device *ec_dev;
+};
+
+/**
+ * ish_evt_handler - ISH to AP event handler
+ * @work: Work struct
+ */
+static void ish_evt_handler(struct work_struct *work)
+{
+ struct ishtp_cl_data *client_data =
+ container_of(work, struct ishtp_cl_data, work_ec_evt);
+ struct cros_ec_device *ec_dev = client_data->ec_dev;
+
+ if (cros_ec_get_next_event(ec_dev, NULL) > 0) {
+ blocking_notifier_call_chain(&ec_dev->event_notifier,
+ 0, ec_dev);
+ }
+}
+
+/**
+ * ish_send() - Send message from host to firmware
+ *
+ * @client_data: Client data instance
+ * @out_msg: Message buffer to be sent to firmware
+ * @out_size: Size of out going message
+ * @in_msg: Message buffer where the incoming data is copied. This buffer
+ * is allocated by calling
+ * @in_size: Max size of incoming message
+ *
+ * Return: Number of bytes copied in the in_msg on success, negative
+ * error code on failure.
+ */
+static int ish_send(struct ishtp_cl_data *client_data,
+ u8 *out_msg, size_t out_size,
+ u8 *in_msg, size_t in_size)
+{
+ int rv;
+ struct header *out_hdr = (struct header *)out_msg;
+ struct ishtp_cl *cros_ish_cl = client_data->cros_ish_cl;
+
+ dev_dbg(cl_data_to_dev(client_data),
+ "%s: channel=%02u status=%02u\n",
+ __func__, out_hdr->channel, out_hdr->status);
+
+ /* Setup for incoming response */
+ client_data->response.data = in_msg;
+ client_data->response.max_size = in_size;
+ client_data->response.error = 0;
+ client_data->response.received = false;
+
+ rv = ishtp_cl_send(cros_ish_cl, out_msg, out_size);
+ if (rv) {
+ dev_err(cl_data_to_dev(client_data),
+ "ishtp_cl_send error %d\n", rv);
+ return rv;
+ }
+
+ wait_event_interruptible_timeout(client_data->response.wait_queue,
+ client_data->response.received,
+ ISHTP_SEND_TIMEOUT);
+ if (!client_data->response.received) {
+ dev_err(cl_data_to_dev(client_data),
+ "Timed out for response to host message\n");
+ return -ETIMEDOUT;
+ }
+
+ if (client_data->response.error < 0)
+ return client_data->response.error;
+
+ return client_data->response.size;
+}
+
+/**
+ * process_recv() - Received and parse incoming packet
+ * @cros_ish_cl: Client instance to get stats
+ * @rb_in_proc: Host interface message buffer
+ *
+ * Parse the incoming packet. If it is a response packet then it will
+ * update per instance flags and wake up the caller waiting to for the
+ * response. If it is an event packet then it will schedule event work.
+ */
+static void process_recv(struct ishtp_cl *cros_ish_cl,
+ struct ishtp_cl_rb *rb_in_proc)
+{
+ size_t data_len = rb_in_proc->buf_idx;
+ struct ishtp_cl_data *client_data =
+ ishtp_get_client_data(cros_ish_cl);
+ struct device *dev = cl_data_to_dev(client_data);
+ struct cros_ish_in_msg *in_msg =
+ (struct cros_ish_in_msg *)rb_in_proc->buffer.data;
+
+ /* Proceed only if reset or init is not in progress */
+ if (!down_read_trylock(&init_lock)) {
+ /* Free the buffer */
+ ishtp_cl_io_rb_recycle(rb_in_proc);
+ dev_warn(dev,
+ "Host is not ready to receive incoming messages\n");
+ return;
+ }
+
+ /*
+ * All firmware messages contain a header. Check the buffer size
+ * before accessing elements inside.
+ */
+ if (!rb_in_proc->buffer.data) {
+ dev_warn(dev, "rb_in_proc->buffer.data returned null");
+ client_data->response.error = -EBADMSG;
+ goto end_error;
+ }
+
+ if (data_len < sizeof(struct header)) {
+ dev_err(dev, "data size %zu is less than header %zu\n",
+ data_len, sizeof(struct header));
+ client_data->response.error = -EMSGSIZE;
+ goto end_error;
+ }
+
+ dev_dbg(dev, "channel=%02u status=%02u\n",
+ in_msg->hdr.channel, in_msg->hdr.status);
+
+ switch (in_msg->hdr.channel) {
+ case CROS_EC_COMMAND:
+ /* Sanity check */
+ if (!client_data->response.data) {
+ dev_err(dev,
+ "Receiving buffer is null. Should be allocated by calling function\n");
+ client_data->response.error = -EINVAL;
+ goto error_wake_up;
+ }
+
+ if (client_data->response.received) {
+ dev_err(dev,
+ "Previous firmware message not yet processed\n");
+ client_data->response.error = -EINVAL;
+ goto error_wake_up;
+ }
+
+ if (data_len > client_data->response.max_size) {
+ dev_err(dev,
+ "Received buffer size %zu is larger than allocated buffer %zu\n",
+ data_len, client_data->response.max_size);
+ client_data->response.error = -EMSGSIZE;
+ goto error_wake_up;
+ }
+
+ if (in_msg->hdr.status) {
+ dev_err(dev, "firmware returned status %d\n",
+ in_msg->hdr.status);
+ client_data->response.error = -EIO;
+ goto error_wake_up;
+ }
+
+ /* Update the actual received buffer size */
+ client_data->response.size = data_len;
+
+ /*
+ * Copy the buffer received in firmware response for the
+ * calling thread.
+ */
+ memcpy(client_data->response.data,
+ rb_in_proc->buffer.data, data_len);
+
+ /* Set flag before waking up the caller */
+ client_data->response.received = true;
+error_wake_up:
+ /* Wake the calling thread */
+ wake_up_interruptible(&client_data->response.wait_queue);
+
+ break;
+
+ case CROS_MKBP_EVENT:
+ /* The event system doesn't send any data in buffer */
+ schedule_work(&client_data->work_ec_evt);
+
+ break;
+
+ default:
+ dev_err(dev, "Invalid channel=%02d\n", in_msg->hdr.channel);
+ }
+
+end_error:
+ /* Free the buffer */
+ ishtp_cl_io_rb_recycle(rb_in_proc);
+
+ up_read(&init_lock);
+}
+
+/**
+ * ish_event_cb() - bus driver callback for incoming message
+ * @cl_device: ISHTP client device for which this message is targeted.
+ *
+ * Remove the packet from the list and process the message by calling
+ * process_recv.
+ */
+static void ish_event_cb(struct ishtp_cl_device *cl_device)
+{
+ struct ishtp_cl_rb *rb_in_proc;
+ struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
+
+ while ((rb_in_proc = ishtp_cl_rx_get_rb(cros_ish_cl)) != NULL) {
+ /* Decide what to do with received data */
+ process_recv(cros_ish_cl, rb_in_proc);
+ }
+}
+
+/**
+ * cros_ish_init() - Init function for ISHTP client
+ * @cros_ish_cl: ISHTP client instance
+ *
+ * This function complete the initializtion of the client.
+ *
+ * Return: 0 for success, negative error code for failure.
+ */
+static int cros_ish_init(struct ishtp_cl *cros_ish_cl)
+{
+ int rv;
+ struct ishtp_device *dev;
+ struct ishtp_fw_client *fw_client;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
+
+ rv = ishtp_cl_link(cros_ish_cl);
+ if (rv) {
+ dev_err(cl_data_to_dev(client_data),
+ "ishtp_cl_link failed\n");
+ return rv;
+ }
+
+ dev = ishtp_get_ishtp_device(cros_ish_cl);
+
+ /* Connect to firmware client */
+ ishtp_set_tx_ring_size(cros_ish_cl, CROS_ISH_CL_TX_RING_SIZE);
+ ishtp_set_rx_ring_size(cros_ish_cl, CROS_ISH_CL_RX_RING_SIZE);
+
+ fw_client = ishtp_fw_cl_get_client(dev, &cros_ish_guid);
+ if (!fw_client) {
+ dev_err(cl_data_to_dev(client_data),
+ "ish client uuid not found\n");
+ rv = -ENOENT;
+ goto err_cl_unlink;
+ }
+
+ ishtp_cl_set_fw_client_id(cros_ish_cl,
+ ishtp_get_fw_client_id(fw_client));
+ ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_CONNECTING);
+
+ rv = ishtp_cl_connect(cros_ish_cl);
+ if (rv) {
+ dev_err(cl_data_to_dev(client_data),
+ "client connect fail\n");
+ goto err_cl_unlink;
+ }
+
+ ishtp_register_event_cb(client_data->cl_device, ish_event_cb);
+ return 0;
+
+err_cl_unlink:
+ ishtp_cl_unlink(cros_ish_cl);
+ return rv;
+}
+
+/**
+ * cros_ish_deinit() - Deinit function for ISHTP client
+ * @cros_ish_cl: ISHTP client instance
+ *
+ * Unlink and free cros_ec client
+ */
+static void cros_ish_deinit(struct ishtp_cl *cros_ish_cl)
+{
+ ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_DISCONNECTING);
+ ishtp_cl_disconnect(cros_ish_cl);
+ ishtp_cl_unlink(cros_ish_cl);
+ ishtp_cl_flush_queues(cros_ish_cl);
+
+ /* Disband and free all Tx and Rx client-level rings */
+ ishtp_cl_free(cros_ish_cl);
+}
+
+/**
+ * prepare_cros_ec_rx() - Check & prepare receive buffer
+ * @ec_dev: CrOS EC MFD device.
+ * @in_msg: Incoming message buffer
+ * @msg: cros_ec command used to send & receive data
+ *
+ * Return: 0 for success, negative error code for failure.
+ *
+ * Check the received buffer. Convert to cros_ec_command format.
+ */
+static int prepare_cros_ec_rx(struct cros_ec_device *ec_dev,
+ const struct cros_ish_in_msg *in_msg,
+ struct cros_ec_command *msg)
+{
+ u8 sum = 0;
+ int i, rv, offset;
+
+ /* Check response error code */
+ msg->result = in_msg->ec_response.result;
+ rv = cros_ec_check_result(ec_dev, msg);
+ if (rv < 0)
+ return rv;
+
+ if (in_msg->ec_response.data_len > msg->insize) {
+ dev_err(ec_dev->dev, "Packet too long (%d bytes, expected %d)",
+ in_msg->ec_response.data_len, msg->insize);
+ return -ENOSPC;
+ }
+
+ /* Copy response packet payload and compute checksum */
+ for (i = 0; i < sizeof(struct ec_host_response); i++)
+ sum += ((u8 *)in_msg)[IN_MSG_EC_RESPONSE_PREAMBLE + i];
+
+ offset = sizeof(struct cros_ish_in_msg);
+ for (i = 0; i < in_msg->ec_response.data_len; i++)
+ sum += msg->data[i] = ((u8 *)in_msg)[offset + i];
+
+ if (sum) {
+ dev_dbg(ec_dev->dev, "Bad received packet checksum %d\n", sum);
+ return -EBADMSG;
+ }
+
+ return 0;
+}
+
+static int cros_ec_pkt_xfer_ish(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *msg)
+{
+ int rv;
+ struct ishtp_cl *cros_ish_cl = ec_dev->priv;
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
+ struct device *dev = cl_data_to_dev(client_data);
+ struct cros_ish_in_msg *in_msg = (struct cros_ish_in_msg *)ec_dev->din;
+ struct cros_ish_out_msg *out_msg =
+ (struct cros_ish_out_msg *)ec_dev->dout;
+ size_t in_size = sizeof(struct cros_ish_in_msg) + msg->insize;
+ size_t out_size = sizeof(struct cros_ish_out_msg) + msg->outsize;
+
+ /* Sanity checks */
+ if (in_size > ec_dev->din_size) {
+ dev_err(dev,
+ "Incoming payload size %zu is too large for ec_dev->din_size %d\n",
+ in_size, ec_dev->din_size);
+ return -EMSGSIZE;
+ }
+
+ if (out_size > ec_dev->dout_size) {
+ dev_err(dev,
+ "Outgoing payload size %zu is too large for ec_dev->dout_size %d\n",
+ out_size, ec_dev->dout_size);
+ return -EMSGSIZE;
+ }
+
+ /* Proceed only if reset-init is not in progress */
+ if (!down_read_trylock(&init_lock)) {
+ dev_warn(dev,
+ "Host is not ready to send messages to ISH. Try again\n");
+ return -EAGAIN;
+ }
+
+ /* Prepare the package to be sent over ISH TP */
+ out_msg->hdr.channel = CROS_EC_COMMAND;
+ out_msg->hdr.status = 0;
+
+ ec_dev->dout += OUT_MSG_EC_REQUEST_PREAMBLE;
+ cros_ec_prepare_tx(ec_dev, msg);
+ ec_dev->dout -= OUT_MSG_EC_REQUEST_PREAMBLE;
+
+ dev_dbg(dev,
+ "out_msg: struct_ver=0x%x checksum=0x%x command=0x%x command_ver=0x%x data_len=0x%x\n",
+ out_msg->ec_request.struct_version,
+ out_msg->ec_request.checksum,
+ out_msg->ec_request.command,
+ out_msg->ec_request.command_version,
+ out_msg->ec_request.data_len);
+
+ /* Send command to ISH EC firmware and read response */
+ rv = ish_send(client_data,
+ (u8 *)out_msg, out_size,
+ (u8 *)in_msg, in_size);
+ if (rv < 0)
+ goto end_error;
+
+ rv = prepare_cros_ec_rx(ec_dev, in_msg, msg);
+ if (rv)
+ goto end_error;
+
+ rv = in_msg->ec_response.data_len;
+
+ dev_dbg(dev,
+ "in_msg: struct_ver=0x%x checksum=0x%x result=0x%x data_len=0x%x\n",
+ in_msg->ec_response.struct_version,
+ in_msg->ec_response.checksum,
+ in_msg->ec_response.result,
+ in_msg->ec_response.data_len);
+
+end_error:
+ if (msg->command == EC_CMD_REBOOT_EC)
+ msleep(EC_REBOOT_DELAY_MS);
+
+ up_read(&init_lock);
+
+ return rv;
+}
+
+static int cros_ec_dev_init(struct ishtp_cl_data *client_data)
+{
+ struct cros_ec_device *ec_dev;
+ struct device *dev = cl_data_to_dev(client_data);
+
+ ec_dev = devm_kzalloc(dev, sizeof(*ec_dev), GFP_KERNEL);
+ if (!ec_dev)
+ return -ENOMEM;
+
+ client_data->ec_dev = ec_dev;
+ dev->driver_data = ec_dev;
+
+ ec_dev->dev = dev;
+ ec_dev->priv = client_data->cros_ish_cl;
+ ec_dev->cmd_xfer = NULL;
+ ec_dev->pkt_xfer = cros_ec_pkt_xfer_ish;
+ ec_dev->phys_name = dev_name(dev);
+ ec_dev->din_size = sizeof(struct cros_ish_in_msg) +
+ sizeof(struct ec_response_get_protocol_info);
+ ec_dev->dout_size = sizeof(struct cros_ish_out_msg);
+
+ return cros_ec_register(ec_dev);
+}
+
+static void reset_handler(struct work_struct *work)
+{
+ int rv;
+ struct device *dev;
+ struct ishtp_cl *cros_ish_cl;
+ struct ishtp_cl_device *cl_device;
+ struct ishtp_cl_data *client_data =
+ container_of(work, struct ishtp_cl_data, work_ishtp_reset);
+
+ /* Lock for reset to complete */
+ down_write(&init_lock);
+
+ cros_ish_cl = client_data->cros_ish_cl;
+ cl_device = client_data->cl_device;
+
+ /* Unlink, flush queues & start again */
+ ishtp_cl_unlink(cros_ish_cl);
+ ishtp_cl_flush_queues(cros_ish_cl);
+ ishtp_cl_free(cros_ish_cl);
+
+ cros_ish_cl = ishtp_cl_allocate(cl_device);
+ if (!cros_ish_cl) {
+ up_write(&init_lock);
+ return;
+ }
+
+ ishtp_set_drvdata(cl_device, cros_ish_cl);
+ ishtp_set_client_data(cros_ish_cl, client_data);
+ client_data->cros_ish_cl = cros_ish_cl;
+
+ rv = cros_ish_init(cros_ish_cl);
+ if (rv) {
+ ishtp_cl_free(cros_ish_cl);
+ dev_err(cl_data_to_dev(client_data), "Reset Failed\n");
+ up_write(&init_lock);
+ return;
+ }
+
+ /* Refresh ec_dev device pointers */
+ client_data->ec_dev->priv = client_data->cros_ish_cl;
+ dev = cl_data_to_dev(client_data);
+ dev->driver_data = client_data->ec_dev;
+
+ dev_info(cl_data_to_dev(client_data), "Chrome EC ISH reset done\n");
+
+ up_write(&init_lock);
+}
+
+/**
+ * cros_ec_ishtp_probe() - ISHTP client driver probe callback
+ * @cl_device: ISHTP client device instance
+ *
+ * Return: 0 for success, negative error code for failure.
+ */
+static int cros_ec_ishtp_probe(struct ishtp_cl_device *cl_device)
+{
+ int rv;
+ struct ishtp_cl *cros_ish_cl;
+ struct ishtp_cl_data *client_data =
+ devm_kzalloc(ishtp_device(cl_device),
+ sizeof(*client_data), GFP_KERNEL);
+ if (!client_data)
+ return -ENOMEM;
+
+ /* Lock for initialization to complete */
+ down_write(&init_lock);
+
+ cros_ish_cl = ishtp_cl_allocate(cl_device);
+ if (!cros_ish_cl) {
+ rv = -ENOMEM;
+ goto end_ishtp_cl_alloc_error;
+ }
+
+ ishtp_set_drvdata(cl_device, cros_ish_cl);
+ ishtp_set_client_data(cros_ish_cl, client_data);
+ client_data->cros_ish_cl = cros_ish_cl;
+ client_data->cl_device = cl_device;
+
+ init_waitqueue_head(&client_data->response.wait_queue);
+
+ INIT_WORK(&client_data->work_ishtp_reset,
+ reset_handler);
+ INIT_WORK(&client_data->work_ec_evt,
+ ish_evt_handler);
+
+ rv = cros_ish_init(cros_ish_cl);
+ if (rv)
+ goto end_ishtp_cl_init_error;
+
+ ishtp_get_device(cl_device);
+
+ up_write(&init_lock);
+
+ /* Register croc_ec_dev mfd */
+ rv = cros_ec_dev_init(client_data);
+ if (rv)
+ goto end_cros_ec_dev_init_error;
+
+ return 0;
+
+end_cros_ec_dev_init_error:
+ ishtp_set_connection_state(cros_ish_cl, ISHTP_CL_DISCONNECTING);
+ ishtp_cl_disconnect(cros_ish_cl);
+ ishtp_cl_unlink(cros_ish_cl);
+ ishtp_cl_flush_queues(cros_ish_cl);
+ ishtp_put_device(cl_device);
+end_ishtp_cl_init_error:
+ ishtp_cl_free(cros_ish_cl);
+end_ishtp_cl_alloc_error:
+ up_write(&init_lock);
+ return rv;
+}
+
+/**
+ * cros_ec_ishtp_remove() - ISHTP client driver remove callback
+ * @cl_device: ISHTP client device instance
+ *
+ * Return: 0
+ */
+static int cros_ec_ishtp_remove(struct ishtp_cl_device *cl_device)
+{
+ struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
+
+ cancel_work_sync(&client_data->work_ishtp_reset);
+ cancel_work_sync(&client_data->work_ec_evt);
+ cros_ish_deinit(cros_ish_cl);
+ ishtp_put_device(cl_device);
+
+ return 0;
+}
+
+/**
+ * cros_ec_ishtp_reset() - ISHTP client driver reset callback
+ * @cl_device: ISHTP client device instance
+ *
+ * Return: 0
+ */
+static int cros_ec_ishtp_reset(struct ishtp_cl_device *cl_device)
+{
+ struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
+
+ schedule_work(&client_data->work_ishtp_reset);
+
+ return 0;
+}
+
+/**
+ * cros_ec_ishtp_suspend() - ISHTP client driver suspend callback
+ * @device: device instance
+ *
+ * Return: 0 for success, negative error code for failure.
+ */
+static int __maybe_unused cros_ec_ishtp_suspend(struct device *device)
+{
+ struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
+ struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
+
+ return cros_ec_suspend(client_data->ec_dev);
+}
+
+/**
+ * cros_ec_ishtp_resume() - ISHTP client driver resume callback
+ * @device: device instance
+ *
+ * Return: 0 for success, negative error code for failure.
+ */
+static int __maybe_unused cros_ec_ishtp_resume(struct device *device)
+{
+ struct ishtp_cl_device *cl_device = dev_get_drvdata(device);
+ struct ishtp_cl *cros_ish_cl = ishtp_get_drvdata(cl_device);
+ struct ishtp_cl_data *client_data = ishtp_get_client_data(cros_ish_cl);
+
+ return cros_ec_resume(client_data->ec_dev);
+}
+
+static SIMPLE_DEV_PM_OPS(cros_ec_ishtp_pm_ops, cros_ec_ishtp_suspend,
+ cros_ec_ishtp_resume);
+
+static struct ishtp_cl_driver cros_ec_ishtp_driver = {
+ .name = "cros_ec_ishtp",
+ .guid = &cros_ish_guid,
+ .probe = cros_ec_ishtp_probe,
+ .remove = cros_ec_ishtp_remove,
+ .reset = cros_ec_ishtp_reset,
+ .driver = {
+ .pm = &cros_ec_ishtp_pm_ops,
+ },
+};
+
+static int __init cros_ec_ishtp_mod_init(void)
+{
+ return ishtp_cl_driver_register(&cros_ec_ishtp_driver, THIS_MODULE);
+}
+
+static void __exit cros_ec_ishtp_mod_exit(void)
+{
+ ishtp_cl_driver_unregister(&cros_ec_ishtp_driver);
+}
+
+module_init(cros_ec_ishtp_mod_init);
+module_exit(cros_ec_ishtp_mod_exit);
+
+MODULE_DESCRIPTION("ChromeOS EC ISHTP Client Driver");
+MODULE_AUTHOR("Rushikesh S Kadam <rushikesh.s.kadam@intel.com>");
+
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("ishtp:*");
--
1.9.1
^ permalink raw reply related
* [PATCH v4 05/13] platform/x86: asus-wmi: Improve DSTS WMI method ID detection
From: Yurii Pavlovskyi @ 2019-05-14 19:00 UTC (permalink / raw)
Cc: Corentin Chary, Darren Hart, Andy Shevchenko, Daniel Drake,
Chris Chiu, acpi4asus-user, platform-driver-x86, linux-kernel,
Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <c8cdb347-e206-76b2-0d43-546ef660ffb7@gmail.com>
The DSTS method detection mistakenly selects DCTS instead of DSTS if
nothing is returned when the method ID is not defined in WMNB. As a result,
the control of keyboard backlight is not functional for TUF Gaming series
laptops. Implement detection based on _UID of the WMI device instead.
There is evidence that DCTS is handled by ACPI WMI devices that have _UID
ASUSWMI, whereas none of the devices without ASUSWMI respond to DCTS and
DSTS is used instead [1].
DSDT examples:
FX505GM (_UID ATK):
Method (WMNB, 3, Serialized)
{ ...
If ((Local0 == 0x53545344))
{
...
Return (Zero)
}
...
// No return
}
K54C (_UID ATK):
Method (WMNB, 3, Serialized)
{ ...
If ((Local0 == 0x53545344))
{
...
Return (0x02)
}
...
Return (0xFFFFFFFE)
}
[1] Link: https://lkml.org/lkml/2019/4/11/322
Signed-off-by: Yurii Pavlovskyi <yurii.pavlovskyi@gmail.com>
Suggested-by: Daniel Drake <drake@endlessm.com>
---
Depends on the previous patch in the series that adds method to
wmi module.
---
drivers/hid/hid-asus.c | 2 +-
drivers/platform/x86/asus-wmi.c | 23 +++++++++++++++++++---
include/linux/platform_data/x86/asus-wmi.h | 4 ++--
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 336aeaed1159..1d01fe23ca0c 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -396,7 +396,7 @@ static bool asus_kbd_wmi_led_control_present(struct hid_device *hdev)
if (!IS_ENABLED(CONFIG_ASUS_WMI))
return false;
- ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS2,
+ ret = asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS,
ASUS_WMI_DEVID_KBD_BACKLIGHT, 0, &value);
hid_dbg(hdev, "WMI backlight check: rc %d value %x", ret, value);
if (ret)
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 6b35c1f00a3e..5bdb4ffdbee3 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -83,6 +83,8 @@ MODULE_LICENSE("GPL");
#define USB_INTEL_XUSB2PR 0xD0
#define PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI 0x9c31
+#define ASUS_ACPI_UID_ASUSWMI "ASUSWMI"
+
static const char * const ashs_ids[] = { "ATK4001", "ATK4002", NULL };
static bool ashs_present(void)
@@ -1874,6 +1876,8 @@ static int asus_wmi_sysfs_init(struct platform_device *device)
*/
static int asus_wmi_platform_init(struct asus_wmi *asus)
{
+ struct device *dev = &asus->platform_device->dev;
+ char *wmi_uid;
int rv;
/* INIT enable hotkeys on some models */
@@ -1903,11 +1907,24 @@ static int asus_wmi_platform_init(struct asus_wmi *asus)
* Note, on most Eeepc, there is no way to check if a method exist
* or note, while on notebooks, they returns 0xFFFFFFFE on failure,
* but once again, SPEC may probably be used for that kind of things.
+ *
+ * Additionally at least TUF Gaming series laptops return nothing for
+ * unknown methods, so the detection in this way is not possible.
+ *
+ * There is strong indication that only ACPI WMI devices that have _UID
+ * equal to "ASUSWMI" use DCTS whereas those with "ATK" use DSTS.
*/
- if (!asus_wmi_evaluate_method(ASUS_WMI_METHODID_DSTS, 0, 0, NULL))
+ wmi_uid = wmi_get_acpi_device_uid(ASUS_WMI_MGMT_GUID);
+ if (!wmi_uid)
+ return -ENODEV;
+
+ if (!strcmp(wmi_uid, ASUS_ACPI_UID_ASUSWMI)) {
+ dev_info(dev, "Detected ASUSWMI, use DCTS\n");
+ asus->dsts_id = ASUS_WMI_METHODID_DCTS;
+ } else {
+ dev_info(dev, "Detected %s, not ASUSWMI, use DSTS\n", wmi_uid);
asus->dsts_id = ASUS_WMI_METHODID_DSTS;
- else
- asus->dsts_id = ASUS_WMI_METHODID_DSTS2;
+ }
/* CWAP allow to define the behavior of the Fn+F2 key,
* this method doesn't seems to be present on Eee PCs */
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index bfba245636a7..0668f76df921 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -18,8 +18,8 @@
#define ASUS_WMI_METHODID_GDSP 0x50534447 /* Get DiSPlay output */
#define ASUS_WMI_METHODID_DEVP 0x50564544 /* DEVice Policy */
#define ASUS_WMI_METHODID_OSVR 0x5256534F /* OS VeRsion */
-#define ASUS_WMI_METHODID_DSTS 0x53544344 /* Device STatuS */
-#define ASUS_WMI_METHODID_DSTS2 0x53545344 /* Device STatuS #2*/
+#define ASUS_WMI_METHODID_DCTS 0x53544344 /* Device status (DCTS) */
+#define ASUS_WMI_METHODID_DSTS 0x53545344 /* Device status (DSTS) */
#define ASUS_WMI_METHODID_BSTS 0x53545342 /* Bios STatuS ? */
#define ASUS_WMI_METHODID_DEVS 0x53564544 /* DEVice Set */
#define ASUS_WMI_METHODID_CFVS 0x53564643 /* CPU Frequency Volt Set */
--
2.17.1
^ permalink raw reply related
* Re: [git pull] Input updates for v5.2-rc0
From: Lukas Wunner @ 2019-05-14 16:30 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linus Torvalds, linux-kernel, linux-input, Ronald Tschalär,
Andy Shevchenko
In-Reply-To: <20190513201235.GA87488@dtor-ws>
On Mon, May 13, 2019 at 01:12:35PM -0700, Dmitry Torokhov wrote:
> Please pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
>
> to receive updates for the input subsystem. You will get a few new
> drivers:
>
> - driver for Azoteq IQS550/572/525 touch controllers
> - driver for Microchip AT42QT1050 keys
> - driver for GPIO controllable vibrators
> - support for GT5663 in Goodix driver
This is missing the driver to support the keyboard and trackpad
on 2015+ MacBooks and MacBook Pros / Airs. Those machines make up
a large segment of the laptop market and are pretty much unusable
on Linux without the driver:
https://www.spinics.net/lists/dri-devel/msg207174.html
The patches have gone through 7 iterations, have been reviewed
extensively, all review comments were addressed properly and
the code is now in excellent shape. The last iteration was
posted to the list 1 month ago, well in time to make it into
this merge window. So I don't understand why it's not part
of this pull request.
Please consider submitting a second pull request for the ongoing
merge window with that driver.
Thanks,
Lukas
^ permalink raw reply
* Re: [PATCH v2 2/4] mfd: ioc3: Add driver for SGI IOC3 chip
From: Thomas Bogendoerfer @ 2019-05-14 14:29 UTC (permalink / raw)
To: Lee Jones
Cc: Ralf Baechle, Paul Burton, James Hogan, Dmitry Torokhov,
David S. Miller, Alessandro Zummo, Alexandre Belloni,
Greg Kroah-Hartman, Jiri Slaby, linux-mips, linux-kernel,
linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190510071419.GB7321@dell>
On Fri, 10 May 2019 08:14:19 +0100
Lee Jones <lee.jones@linaro.org> wrote:
> On Thu, 09 May 2019, Thomas Bogendoerfer wrote:
> > > > + }
> > > > + pr_err("ioc3: CRC error in NIC address\n");
> > > > +}
> > >
> > > This all looks like networking code. If this is the case, it should
> > > be moved to drivers/networking or similar.
> >
> > no it's not. nic stands for number in a can produced by Dallas Semi also
> > known under the name 1-Wire (https://en.wikipedia.org/wiki/1-Wire).
> > SGI used them to provide partnumber, serialnumber and mac addresses.
> > By placing the code to read the NiCs inside ioc3 driver there is no need
> > for locking and adding library code for accessing these informations.
>
> Great. So it looks like you should be using this, no?
>
> drivers/base/regmap/regmap-w1.c
not sure about regmap-w1, but drivers/w1 contains usefull stuff
like w1_crc8. The only drawback I see right now is, that I need
information about part numbers at ioc3 probe time, but for using
w1 framework I need to create a platform device, which will give
me this information not synchronous. I'll see how I could solve that.
> > > > +static struct resource ioc3_uarta_resources[] = {
> > > > + DEFINE_RES_MEM(offsetof(struct ioc3, sregs.uarta),
> > >
> > > You are the first user of offsetof() in MFD. Could you tell me why
> > > it's required please?
> >
> > to get the offsets of different chip functions out of a struct.
>
> I can see what it does on a coding level.
>
> What are you using it for in practical/real terms?
ioc3 has one PCI bar, where all different functions are accessible.
The current ioc3 register map has all these functions set up in one
struct. The base address of these registers comes out of the PCI
framework and to use the MFD framework I need offsets for the different
functions. And because there was already struct ioc3 I'm using
offsetof on this struct.
> Why wouldn't any other MFD driver require this, but you do?
the other PCI MFD drivers I've looked at, have a PCI BAR per function,
which makes live easier and no need for offsetof. Other MFD drivers
#define the offsets and don't have a big struct, which contains all
function registers. If you really insist on using #defines I need
to go through a few parts of the kernel where struct ioc3 is still used.
> > > > + if (ipd->info->funcs & IOC3_SER) {
> > > > + writel(GPCR_UARTA_MODESEL | GPCR_UARTB_MODESEL,
> > > > + &ipd->regs->gpcr_s);
> > > > + writel(0, &ipd->regs->gppr[6]);
> > > > + writel(0, &ipd->regs->gppr[7]);
> > > > + udelay(100);
> > > > + writel(readl(&ipd->regs->port_a.sscr) & ~SSCR_DMA_EN,
> > > > + &ipd->regs->port_a.sscr);
> > > > + writel(readl(&ipd->regs->port_b.sscr) & ~SSCR_DMA_EN,
> > > > + &ipd->regs->port_b.sscr);
> > > > + udelay(1000);
> > >
> > > No idea what any of this does.
> > >
> > > It looks like it belongs in the serial driver (and needs comments).
> >
> > it configures the IOC3 chip for serial usage. This is not part of
> > the serial register set, so it IMHO belongs in the MFD driver.
>
> So it does serial things, but doesn't belong in the serial driver?
It sets up IOC3 GPIOs and IOC3 serial mode in way the 8250 driver
can work with the connected superio.
> Could you please go into a bit more detail as to why you think that?
>
> Why is it better here?
access to gpio and serial mode is outside of the 8250 register space.
So either I need to export with some additional resources/new special
platform data or just set it where it is done.
> It's also totally unreadable by the way!
sure, I'll add comments.
> > > > + }
> > > > +#if defined(CONFIG_SGI_IP27)
> > >
> > > What is this? Can't you obtain this dynamically by probing the H/W?
> >
> > that's the machine type and the #ifdef CONFIG_xxx are just for saving space,
> > when compiled for other machines and it's easy to remove.
>
> Please find other ways to save the space. #ifery can get very messy,
> very quickly and is almost always avoidable.
space isn't a problem at all, so removing #ifdef CONFIG is easy.
>
> > > > + if (ipd->info->irq_offset) {
> > >
> > > What does this really signify?
> >
> > IOC3 ASICs are most of the time connected to a SGI bridge chip. IOC3 can
> > provide two interrupt lines, which are wired to the bridge chip. The first
> > interrupt is assigned via the PCI core, but since IOC3 is not a PCI multi
> > function device the second interrupt must be treated here. And the used
> > interrupt line on the bridge chip differs between boards.
>
> Please provide a MACRO, function or something else which results in
> more readable code. Whatever you choose to use, please add this text
> above, it will be helpful for future readers.
will do.
Thomas.
--
SUSE Linux GmbH
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: [PATCH v2 2/4] mfd: ioc3: Add driver for SGI IOC3 chip
From: Esben Haabendal @ 2019-05-14 12:39 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Lee Jones, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, David S. Miller, Alessandro Zummo,
Alexandre Belloni, Greg Kroah-Hartman, Jiri Slaby, linux-mips,
linux-kernel, linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190409154610.6735-3-tbogendoerfer@suse.de>
On Tue, 09 Apr 2019, Thomas Bogendoerfer wrote:
>
> diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
> index 358e66b81926..21fe722ebcd8 100644
> --- a/drivers/net/ethernet/sgi/ioc3-eth.c
> +++ b/drivers/net/ethernet/sgi/ioc3-eth.c
>
> [ ... ]
>
> - err = pci_request_regions(pdev, "ioc3");
Why are you dropping the call to pci_request_regions()? Shouldn't you
do something similar in the new mfd driver?
When you are use the the BAR 0 resource as mem_base argument to
mfd_add_devices() later on, it will be split into child resources for
the child devices, but they will not be related to the IORESOURCE_MEM
root tree (iomem_resource) anymore. I don't think that is how it is
supposed to be done, as it will allow random other drivers to request
the exact same memory area.
How/where is the memory resources inserted in the root IORESOURCE_MEM
resource (iomem_resource)? Or is it allowed to use resources without
inserting it into the root tree?
> + SET_NETDEV_DEV(dev, &pdev->dev);
> + ip = netdev_priv(dev);
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + ip->regs = ioremap(r->start, resource_size(r));
> + r = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + ip->ssram = ioremap(r->start, resource_size(r));
Maybe use devm_platform_ioremap_resource() instead, which handles both
platform_get_resource() and ioremap() in one call..
/Esben
^ permalink raw reply
* Re: [PATCH V1] elan_i2c: Increment wakeup count if wake source.
From: Dmitry Torokhov @ 2019-05-13 23:29 UTC (permalink / raw)
To: Ravi Chandra Sadineni
Cc: 廖崇榮, Benjamin Tissoires, Abhishek Bhardwaj,
Todd Broch, lkml, linux-input@vger.kernel.org
In-Reply-To: <20190513220610.177489-1-ravisadineni@chromium.org>
Hi Ravi,
On Mon, May 13, 2019 at 3:06 PM Ravi Chandra Sadineni
<ravisadineni@chromium.org> wrote:
>
> Notify the PM core that this dev is the wake source. This helps
> userspace daemon tracking the wake source to identify the origin of the
> wake.
I wonder if we could do that form the i2c core instead of individual drivers?
>
> Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
> ---
> drivers/input/mouse/elan_i2c_core.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index f9525d6f0bfe..2c0561e20b7f 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -981,6 +981,8 @@ static irqreturn_t elan_isr(int irq, void *dev_id)
> if (error)
> goto out;
>
> + pm_wakeup_event(dev, 0);
> +
> switch (report[ETP_REPORT_ID_OFFSET]) {
> case ETP_REPORT_ID:
> elan_report_absolute(data, report);
> --
> 2.20.1
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [git pull] Input updates for v5.2-rc0
From: pr-tracker-bot @ 2019-05-13 22:55 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Linus Torvalds, linux-kernel, linux-input
In-Reply-To: <20190513201235.GA87488@dtor-ws>
The pull request you sent on Mon, 13 May 2019 13:12:35 -0700:
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/0aed4b28187078565cafbfe86b62f941d580d840
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* [PATCH V1] elan_i2c: Increment wakeup count if wake source.
From: Ravi Chandra Sadineni @ 2019-05-13 22:06 UTC (permalink / raw)
To: dmitry.torokhov, ravisadineni, kt.liao, benjamin.tissoires,
abhishekbh, tbroch, linux-kernel, linux-input
Notify the PM core that this dev is the wake source. This helps
userspace daemon tracking the wake source to identify the origin of the
wake.
Signed-off-by: Ravi Chandra Sadineni <ravisadineni@chromium.org>
---
drivers/input/mouse/elan_i2c_core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
index f9525d6f0bfe..2c0561e20b7f 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -981,6 +981,8 @@ static irqreturn_t elan_isr(int irq, void *dev_id)
if (error)
goto out;
+ pm_wakeup_event(dev, 0);
+
switch (report[ETP_REPORT_ID_OFFSET]) {
case ETP_REPORT_ID:
elan_report_absolute(data, report);
--
2.20.1
^ permalink raw reply related
* [git pull] Input updates for v5.2-rc0
From: Dmitry Torokhov @ 2019-05-13 20:12 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
to receive updates for the input subsystem. You will get a few new
drivers:
- driver for Azoteq IQS550/572/525 touch controllers
- driver for Microchip AT42QT1050 keys
- driver for GPIO controllable vibrators
- support for GT5663 in Goodix driver
along with miscellaneous driver fixes.
Changelog:
---------
Andy Shevchenko (2):
Input: edt-ft5x06 - enable ACPI enumeration
Input: edt-ft5x06 - convert to use SPDX identifier
Anson Huang (1):
Input: snvs_pwrkey - use dev_pm_set_wake_irq() to simplify code
Dmitry Torokhov (3):
Input: i8042 - signal wakeup from atkbd/psmouse
Input: add KEY_KBD_LAYOUT_NEXT
HID: input: add mapping for KEY_KBD_LAYOUT_NEXT
Gustavo A. R. Silva (2):
Input: evdev - use struct_size() in kzalloc() and vzalloc()
Input: libps2 - mark expected switch fall-through
Jagan Teki (2):
Input: goodix - add regulators suppot
Input: goodix - add GT5663 CTP support
Jean Delvare (1):
Input: olpc_apsp - depend on ARCH_MMP
Jeff LaBundy (1):
Input: add support for Azoteq IQS550/572/525
Joseph Salisbury (1):
Input: hyperv-keyboard - add module description
Luca Weiss (1):
Input: add a driver for GPIO controllable vibrators
Marco Felsch (1):
Input: qt1050 - add Microchip AT42QT1050 support
Philipp Zabel (2):
Input: synaptics-rmi4 - fill initial format
Input: synaptics-rmi4 - fix enum_fmt
Vladimir Zapolskiy (1):
Input: lpc32xx-key - add clocks property and fix DT binding example
Ziping Chen (1):
Input: sun4i-a10-lradc-keys - add support for A83T
Diffstat:
--------
.../devicetree/bindings/input/gpio-vibrator.yaml | 37 +
.../devicetree/bindings/input/lpc32xx-key.txt | 5 +-
.../devicetree/bindings/input/microchip,qt1050.txt | 78 ++
.../devicetree/bindings/input/sun4i-lradc-keys.txt | 6 +-
.../bindings/input/touchscreen/goodix.txt | 3 +
.../bindings/input/touchscreen/iqs5xx.txt | 80 ++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
drivers/hid/hid-input.c | 2 +
drivers/input/evdev.c | 7 +-
drivers/input/keyboard/Kconfig | 11 +
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/atkbd.c | 2 +
drivers/input/keyboard/qt1050.c | 598 +++++++++++
drivers/input/keyboard/snvs_pwrkey.c | 30 +-
drivers/input/keyboard/sun4i-lradc-keys.c | 38 +-
drivers/input/misc/Kconfig | 12 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/gpio-vibra.c | 207 ++++
drivers/input/mouse/psmouse-base.c | 2 +
drivers/input/rmi4/rmi_f54.c | 21 +-
drivers/input/serio/Kconfig | 1 +
drivers/input/serio/hyperv-keyboard.c | 2 +
drivers/input/serio/i8042.c | 3 -
drivers/input/serio/libps2.c | 1 +
drivers/input/touchscreen/Kconfig | 10 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/edt-ft5x06.c | 23 +-
drivers/input/touchscreen/goodix.c | 54 +
drivers/input/touchscreen/iqs5xx.c | 1133 ++++++++++++++++++++
include/uapi/linux/input-event-codes.h | 1 +
30 files changed, 2297 insertions(+), 74 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/gpio-vibrator.yaml
create mode 100644 Documentation/devicetree/bindings/input/microchip,qt1050.txt
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/iqs5xx.txt
create mode 100644 drivers/input/keyboard/qt1050.c
create mode 100644 drivers/input/misc/gpio-vibra.c
create mode 100644 drivers/input/touchscreen/iqs5xx.c
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH 2/2] input: keyboard: mtk-pmic-keys: add MT6392 support
From: Fabien Parent @ 2019-05-13 14:21 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, mark.rutland, matthias.bgg
Cc: linux-input, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, Fabien Parent
In-Reply-To: <20190513142120.6527-1-fparent@baylibre.com>
Add support for MT6392 PMIC's keys.
Signed-off-by: Fabien Parent <fparent@baylibre.com>
---
drivers/input/keyboard/mtk-pmic-keys.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
index 8e6ebab05ab4..aaf68cbf7e5b 100644
--- a/drivers/input/keyboard/mtk-pmic-keys.c
+++ b/drivers/input/keyboard/mtk-pmic-keys.c
@@ -18,6 +18,7 @@
#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/mfd/mt6323/registers.h>
+#include <linux/mfd/mt6392/registers.h>
#include <linux/mfd/mt6397/core.h>
#include <linux/mfd/mt6397/registers.h>
#include <linux/module.h>
@@ -83,6 +84,16 @@ static const struct mtk_pmic_regs mt6323_regs = {
.pmic_rst_reg = MT6323_TOP_RST_MISC,
};
+static const struct mtk_pmic_regs mt6392_regs = {
+ .keys_regs[MTK_PMIC_PWRKEY_INDEX] =
+ MTK_PMIC_KEYS_REGS(MT6392_CHRSTATUS,
+ 0x2, MT6392_INT_MISC_CON, 0x10),
+ .keys_regs[MTK_PMIC_HOMEKEY_INDEX] =
+ MTK_PMIC_KEYS_REGS(MT6392_CHRSTATUS,
+ 0x4, MT6392_INT_MISC_CON, 0x8),
+ .pmic_rst_reg = MT6392_TOP_RST_MISC,
+};
+
struct mtk_pmic_keys_info {
struct mtk_pmic_keys *keys;
const struct mtk_pmic_keys_regs *regs;
@@ -238,6 +249,9 @@ static const struct of_device_id of_mtk_pmic_keys_match_tbl[] = {
}, {
.compatible = "mediatek,mt6323-keys",
.data = &mt6323_regs,
+ }, {
+ .compatible = "mediatek,mt6392-keys",
+ .data = &mt6392_regs,
}, {
/* sentinel */
}
--
2.20.1
^ permalink raw reply related
* [PATCH 1/2] dt-bindings: input: mtk-pmic-keys: add MT6392 binding definition
From: Fabien Parent @ 2019-05-13 14:21 UTC (permalink / raw)
To: dmitry.torokhov, robh+dt, mark.rutland, matthias.bgg
Cc: linux-input, devicetree, linux-arm-kernel, linux-mediatek,
linux-kernel, Fabien Parent
Add the binding documentation of the mtk-pmic-keys for the MT6392 PMICs.
Signed-off-by: Fabien Parent <fparent@baylibre.com>
---
.../devicetree/bindings/input/mtk-pmic-keys.txt | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/mtk-pmic-keys.txt b/Documentation/devicetree/bindings/input/mtk-pmic-keys.txt
index 2888d07c2ef0..858f78e7262c 100644
--- a/Documentation/devicetree/bindings/input/mtk-pmic-keys.txt
+++ b/Documentation/devicetree/bindings/input/mtk-pmic-keys.txt
@@ -1,15 +1,18 @@
-MediaTek MT6397/MT6323 PMIC Keys Device Driver
+MediaTek MT6397/MT6392/MT6323 PMIC Keys Device Driver
-There are two key functions provided by MT6397/MT6323 PMIC, pwrkey
+There are two key functions provided by MT6397/MT6392/MT6323 PMIC, pwrkey
and homekey. The key functions are defined as the subnode of the function
node provided by MT6397/MT6323 PMIC that is being defined as one kind
of Muti-Function Device (MFD)
-For MT6397/MT6323 MFD bindings see:
+For MT6397/MT6392/MT6323 MFD bindings see:
Documentation/devicetree/bindings/mfd/mt6397.txt
Required properties:
-- compatible: "mediatek,mt6397-keys" or "mediatek,mt6323-keys"
+- compatible: Should be one of:
+ - "mediatek,mt6397-keys"
+ - "mediatek,mt6392-keys"
+ - "mediatek,mt6323-keys"
- linux,keycodes: See Documentation/devicetree/bindings/input/keys.txt
Optional Properties:
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] HID: fix A4Tech horizontal scrolling
From: Błażej Szczygieł @ 2019-05-12 20:59 UTC (permalink / raw)
To: Peter Hutterer
Cc: Benjamin Tissoires, Igor Kushnir, Jiri Kosina,
open list:HID CORE LAYER, lkml
In-Reply-To: <20190507050150.GA9838@jelly>
Hi,
On 07.05.2019 at 07:01, Peter Hutterer wrote:
> On Fri, May 03, 2019 at 01:59:23PM +0200, Benjamin Tissoires wrote:
>> Hi,
>>
>> On Fri, May 3, 2019 at 11:43 AM Igor Kushnir <igorkuo@gmail.com> wrote:
>>>
>>> Hi Benjamin,
>>>
>>> On 5/3/19 10:36 AM, Benjamin Tissoires wrote:
>>>> Hi,
>>>>
>>>> On Thu, May 2, 2019 at 11:37 PM Błażej Szczygieł <spaz16@wp.pl> wrote:
>>>>>
>>>>> Since recent high resolution scrolling changes the A4Tech driver must
>>>>> check for the "REL_WHEEL_HI_RES" usage code.
>>>>>
>>>>> Fixes: 2dc702c991e3774af9d7ce410eef410ca9e2357e (HID: input: use the
>>>>> Resolution Multiplier for high-resolution scrolling)
>>>>>
>>>>> Signed-off-by: Błażej Szczygieł <spaz16@wp.pl>
>>>>
>>>> Thanks for the patch. I do not doubt this fixes the issues, but I
>>>> still wonder if we should not export REL_HWHEEL_HI_RES instead of
>>>> REL_HWHEEL events.
>>>
>>>
>>> If you mean exporting REL_HWHEEL_HI_RES instead of REL_HWHEEL from
>>> hid-a4tech.c, then it makes sense to me, though I do not know the code
>>> well enough to be certain.
>>
>> Yep, that's what I meant. I am worried that userspace doesn't know
>> well how to deal with a device that mixes the new and old REL_WHEEL
>> events.
>
> sorry, I'm not sure what you mean here. The new events are always mixed with
> the old ones anyway, and both should be treated as separate event streams.
> The kernel interface to userspace is fairly easy to deal with, it's the rest
> that's a bit of mess.
>
> [..]
>
>>>
>>
>> OK, thanks both of you for your logs, this is helpful.
>> So just in case I need to come back later, the horizontal wheel is
>> "just" the normal wheel plus a modifier in the report.
>>
>> Anyway, ideally, can we have a v2 of the patch with the 2 changes
>> requested above in the commit message and the introduction of
>> REL_HWHEEL_HI_RES events in addition to REL_HWHEEL?
>> REL_HWHEEL_HI_RES should report `120*value` and we should also keep
>> the reporting of REL_WHEEL as it is currently.
>>
>> Peter, I grepped in the hid code, and it seems hid-cypress.c is having
>> the exact same issue. Sigh.
>
> yeah, I found that too when grepping through it. seems to be the only other
> one though and we can use Błażej's patch as boilerplate once it's done.
Peter, I also found comparison of "usage->code ==" with "REL_HWHEEL"
and "REL_WHEEL" in hid-lenovo.c, hid-apple.c, hid-ezkey.c, hid-lg.c.
Unfortunatelly, I don't have such devices to test :(
Cheers,
Błażej.
^ permalink raw reply
* [PATCH v3] HID: fix A4Tech horizontal scrolling
From: Błażej Szczygieł @ 2019-05-12 20:33 UTC (permalink / raw)
Cc: igorkuo, peter.hutterer, Błażej Szczygieł,
Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20190507050029.GA5197@jelly>
Since recent high resolution scrolling changes the A4Tech driver must
check for the "REL_WHEEL_HI_RES" usage code.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=203369
Fixes: 2dc702c991e3774af9d7ce410eef410ca9e2357e ("HID: input: use the
Resolution Multiplier for high-resolution scrolling")
Signed-off-by: Błażej Szczygieł <spaz16@wp.pl>
---
Changes in v2:
- changed commit message
Changes in v3:
- send also high resolution events
drivers/hid/hid-a4tech.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-a4tech.c b/drivers/hid/hid-a4tech.c
index 9428ea7cdf8a..c3a6ce3613fe 100644
--- a/drivers/hid/hid-a4tech.c
+++ b/drivers/hid/hid-a4tech.c
@@ -38,8 +38,10 @@ static int a4_input_mapped(struct hid_device *hdev, struct hid_input *hi,
{
struct a4tech_sc *a4 = hid_get_drvdata(hdev);
- if (usage->type == EV_REL && usage->code == REL_WHEEL)
+ if (usage->type == EV_REL && usage->code == REL_WHEEL_HI_RES) {
set_bit(REL_HWHEEL, *bit);
+ set_bit(REL_HWHEEL_HI_RES, *bit);
+ }
if ((a4->quirks & A4_2WHEEL_MOUSE_HACK_7) && usage->hid == 0x00090007)
return -1;
@@ -60,7 +62,7 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
input = field->hidinput->input;
if (a4->quirks & A4_2WHEEL_MOUSE_HACK_B8) {
- if (usage->type == EV_REL && usage->code == REL_WHEEL) {
+ if (usage->type == EV_REL && usage->code == REL_WHEEL_HI_RES) {
a4->delayed_value = value;
return 1;
}
@@ -68,6 +70,8 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
if (usage->hid == 0x000100b8) {
input_event(input, EV_REL, value ? REL_HWHEEL :
REL_WHEEL, a4->delayed_value);
+ input_event(input, EV_REL, value ? REL_HWHEEL_HI_RES :
+ REL_WHEEL_HI_RES, a4->delayed_value * 120);
return 1;
}
}
@@ -77,8 +81,9 @@ static int a4_event(struct hid_device *hdev, struct hid_field *field,
return 1;
}
- if (usage->code == REL_WHEEL && a4->hw_wheel) {
+ if (usage->code == REL_WHEEL_HI_RES && a4->hw_wheel) {
input_event(input, usage->type, REL_HWHEEL, value);
+ input_event(input, usage->type, REL_HWHEEL_HI_RES, value * 120);
return 1;
}
--
2.21.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox